Project

General

Profile

Actions

Bug #2353

closed

hash.c:setenv causes crashes in Solaris

Added by docwhat (Christian Höltje) over 15 years ago. Updated about 14 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 1.9.2dev (2009-11-10 trunk 25704) [sparc-solaris2.8]
Backport:
[ruby-core:26668]

Description

=begin
Hi!

I submitted this as a pull request for shyouhei's ruby git repo, via github.
I want to document this here too.

The environ manipulations that are done in the fall-back code in hash.c (the code that runs if !WIN32 and !HAVE_SETENV) causes crashes in Solaris some times. In addition, it fails hard if you use the binary on a newer version of Solaris.

The code I wrote works on Solaris 8 (all tests pass) and works when the binary is copied to a Solaris 10 system and some simple test code that sets and unsets lots of environment variable is run.

With a clean ruby, the test cases pass in Solaris 8 (usually) but my test code would not run at all when the binary is copied to Solaris 10.

If there is anything else I should do, please don't hesitant to tell me.

Ciao!
=end


Files

solaris_setenv.patch (2.13 KB) solaris_setenv.patch Patch v1 docwhat (Christian Höltje), 11/11/2009 04:50 AM
test_setenv.rb (1.38 KB) test_setenv.rb docwhat (Christian Höltje), 11/12/2009 12:57 AM
Actions #1

Updated by docwhat (Christian Höltje) over 15 years ago

=begin
I didn't think of this, but the patch can also apply to 1.8.7. The line numbers may differ, but the code is the same.

I'm not sure where the 1.8.7 branch is in git or how to submit to that. I'd be happy to submit that to Shyouhei as well.

Ciao!
=end

Actions #2

Updated by nobu (Nobuyoshi Nakada) over 15 years ago

=begin
Hi,

At Wed, 11 Nov 2009 04:50:33 +0900,
Christian Höltje wrote in [ruby-core:26668]:

The environ manipulations that are done in the fall-back code
in hash.c (the code that runs if !WIN32 and !HAVE_SETENV)
causes crashes in Solaris some times. In addition, it fails
hard if you use the binary on a newer version of Solaris.

What does this note mean?

  * Note: This does leak memory and there isn't anything we can do
  * about it.

putenv()ed string is stored in environ but isn't freed?

--
Nobu Nakada

=end

Actions #3

Updated by docwhat (Christian Höltje) over 15 years ago

=begin
Because new variables are allocated by ALLOC_N() but the unsetenv code cannot de-allocate the memory: we don't know if it is initially allocated memory by the kernel (which I think is stack memory and absolutely cannot be free()d) or if it is something we did a putenv() on (which we cannot free() either, depending on the platform).

Ciao!
=end

Actions #4

Updated by nobu (Nobuyoshi Nakada) over 15 years ago

=begin
Hi,

At Wed, 11 Nov 2009 11:32:37 +0900,
Christian Höltje wrote in [ruby-core:26675]:

Because new variables are allocated by ALLOC_N() but the
unsetenv code cannot de-allocate the memory: we don't know if
it is initially allocated memory by the kernel (which I think
is stack memory and absolutely cannot be free()d) or if it is
something we did a putenv() on (which we cannot free()
either, depending on the platform).

Does this work?


Index: hash.c

--- hash.c (revision 25717)
+++ hash.c (working copy)
@@ -2012,5 +2012,16 @@ rb_env_path_tainted(void)
}

-#if !defined(_WIN32) && !(defined(HAVE_SETENV) && defined(HAVE_UNSETENV))
+#if defined(_WIN32) || (defined(HAVE_SETENV) && defined(HAVE_UNSETENV))
+#elif defined sun
+static int
+in_origenv(const char *str)
+{

  • char **env;
  • for (env = origenviron; *env; ++env) {
  • if (*env == str) return 1;
  • }
  • return 0;
    +}
    +#else
    static int
    envix(const char *nam)
    @@ -2058,4 +2069,19 @@ ruby_setenv(const char *name, const char
    else
    unsetenv(name);
    +#elif defined sun
  • size_t len = strlen(name);
  • char **env_ptr, *str;
  • for (env_ptr = GET_ENVIRON(environ); (str = *env_ptr) != 0; ++env_ptr) {
  • if (!strncmp(str, name, len) && str[len] == '=') {
  •  if (!in_origenv(str)) free(str);
    
  •  while ((env_ptr[0] = env_ptr[1]) != 0) env_ptr++;
    
  •  break;
    
  • }
  • }
  • if (value) {
  • str = malloc(len += strlen(value) + 2);
  • snprintf(str, len, "%s=%s", name, value);
  • putenv(str);
  • }
    #else /* WIN32 */
    size_t len;

--
Nobu Nakada

=end

Actions #5

Updated by docwhat (Christian Höltje) over 15 years ago

=begin
It works. I've tested it with my simplistic test case on both "Solaris 8" and "Solaris 10 using the Solaris 8 binary".

It makes me nervous to try to reclaim the memory, but I can't find any fault in the code.

I'm attaching my lame-o test script. It's invoked with a number which represents the number of variables to add/delete.

BTW: Here is the man page for Solaris 10's putenv: http://docs.sun.com/app/docs/doc/816-5168/putenv-3c?a=view

Ciao!
=end

Actions #6

Updated by docwhat (Christian Höltje) over 15 years ago

=begin
I'm not sure the reclaiming is actually working. I ran some tests and it at least appears that it may not be reclaiming much or any. I'm not entirely sure how to test that.

I think I would recommend using the original style unsetenv code I used and just live with the memory leak. It's not usually that much and is usually done right before an exec()...

=end

Actions #7

Updated by nobu (Nobuyoshi Nakada) over 15 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

=begin
This issue was solved with changeset r25766.
Christian, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0