Skip to content

Commit

Permalink
Fix leak reported by Valgrind when setting environment variables
Browse files Browse the repository at this point in the history
The call to setenv replaces the string in the environ array, which causes
the original string to be reported as a memory leak.

This commit allocates another copy of environ called alloc_environ that
contains the strings allocated by ruby_init_setproctitle. Then in
ruby_free_proctitle it frees all the strings in alloc_environ.

For example:

    ENV.each { |k, v| ENV[k] = v.dup }

Valgrind reports:

    3,321 bytes in 39 blocks are definitely lost in loss record 3 of 3
      at 0x484884F: malloc (vg_replace_malloc.c:393)
      by 0x25CB5B: objspace_xmalloc0 (gc.c:11972)
      by 0x40344E: ruby_strdup (util.c:540)
      by 0x59C854: ruby_init_setproctitle (setproctitle.c:143)
      by 0x38CC44: ruby_process_options (ruby.c:3101)
      by 0x234DB1: ruby_options (eval.c:117)
      by 0x15B92E: rb_main (main.c:40)
      by 0x15B92E: main (main.c:59)
  • Loading branch information
peterzhu2118 committed Apr 30, 2024
1 parent d7ba0fe commit e5c0196
Showing 1 changed file with 13 additions and 12 deletions.
25 changes: 13 additions & 12 deletions missing/setproctitle.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ static char **argv1_addr = NULL;

#if ALLOCATE_ENVIRON
static char **orig_environ = NULL;
static char **alloc_environ = NULL;
#endif

void
Expand All @@ -112,7 +113,10 @@ compat_init_setproctitle(int argc, char *argv[])
/* Fail if we can't allocate room for the new environment */
for (i = 0; envp[i] != NULL; i++);

orig_environ = environ = xcalloc(i + 1, sizeof(*environ));
orig_environ = environ;

alloc_environ = xcalloc(i + 1, sizeof(*environ));
environ = xcalloc(i + 1, sizeof(*environ));
if (environ == NULL) {
environ = envp; /* put it back */
return;
Expand Down Expand Up @@ -140,8 +144,8 @@ compat_init_setproctitle(int argc, char *argv[])
argv_env_len = lastenvp - argv[0];

for (i = 0; envp[i] != NULL; i++)
environ[i] = ruby_strdup(envp[i]);
environ[i] = NULL;
alloc_environ[i] = environ[i] = ruby_strdup(envp[i]);
alloc_environ[i] = environ[i] = NULL;
#endif /* SPT_REUSEARGV */
}

Expand All @@ -153,16 +157,13 @@ ruby_free_proctitle(void)

if (!orig_environ) return; /* environ is allocated by OS */

/* ruby_setenv could allocate a new environ, so we need to free orig_environ
* in that case. */
if (environ != orig_environ) {
for (int i = 0; orig_environ[i] != NULL; i++) {
xfree(orig_environ[i]);
}

xfree(orig_environ);
orig_environ = NULL;
for (int i = 0; alloc_environ[i] != NULL; i++) {
xfree(alloc_environ[i]);
}
xfree(alloc_environ);
xfree(environ);

environ = orig_environ;
#endif
}

Expand Down

0 comments on commit e5c0196

Please sign in to comment.