Skip to content

Commit

Permalink
Avoid deadlock with PERL_MEM_LOG
Browse files Browse the repository at this point in the history
This fixes GH #18341

getenv() call allocates memory to squirrel safely away the result of
that getenv() call.  It does this while in a critical section so as to
make sure another thread can't interrupt it and destroy it.

The problem when using PERL_MEM_LOG is that the allocation of that
memory causes a recursive call to getenv() to see how to log that
allocation.  And it deadlocks trying to enter the critical section.

There are various solutions.  One is to use or emulate a general semaphore
instead of a binary one.  This is effectively what
PL_lc_numeric_mutex_depth does for another mutex, and the code for that
could be used as a template.

But given that this is an extreme edge case which requires Perl to be
specially compiled to enable this feature which is used only for
debugging, I thought it would be sufficient to just make a special case
to not try to lock during that recursive call.
  • Loading branch information
khwilliamson committed Nov 26, 2020
1 parent 24d9bd5 commit 89b9a4c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
12 changes: 11 additions & 1 deletion inline.h
Expand Up @@ -2601,7 +2601,17 @@ Perl_mortal_getenv(const char * str)
/* Can't mortalize without stacks. khw believes that no other threads
* should be running, so no need to lock things, and this may be during a
* phase when locking isn't even available */
if (UNLIKELY(PL_scopestack_ix == 0)) {
if ( UNLIKELY(PL_scopestack_ix == 0)

#ifdef PERL_MEM_LOG
/* This is a kludge to prevent deadlock. The process of creating a
* copy likely will call ourselves recursively to get the value of
* PERL_MEM_LOG to see about logging the memory used by that copy.
* Instead just return an unsafe version of it. */
|| UNLIKELY(strEQ(str, "PERL_MEM_LOG"))
#endif

) {
return getenv(str);
}

Expand Down
3 changes: 3 additions & 0 deletions util.c
Expand Up @@ -5008,6 +5008,9 @@ S_mem_log_common(enum mem_log_type mlt, const UV n,

PERL_ARGS_ASSERT_MEM_LOG_COMMON;

/* Note that 'pmlenv' is not protected from other threads overwriting it by
* calling getenv() themselves on platforms where getenv() returns an
* internal static pointer. */
pmlenv = PerlEnv_getenv("PERL_MEM_LOG");
if (!pmlenv)
return;
Expand Down

0 comments on commit 89b9a4c

Please sign in to comment.