Bug fix in malloc.c

[p5pRT]

Migrated from rt.perl.org#50856 (status was 'stalled')

Searchable as RT50856$

[p5pRT]

From mnoyes@brixnet.com

Below is a diff to malloc.c\, in use in perl 5.6.1 and 5.8.8 that fixes a memory management bug in getpages_adjacent() that was encountered when running perl in a multi-threaded environment on Solaris. This change is essentially a replication of code from getpages(). The #if 1 conditional is provided solely to highlight the change.

--- malloc.c +++ malloc.c @​@​ -1837\,6 +1837\,7 @​@​   sbrked_remains = 0;   last_sbrk_top = cp + require;   } else { + MEM_SIZE slack = 0;   if (cp == (char*)-1) { /* Out of memory */ #ifdef DEBUGGING_MSTATS   goodsbrk -= require; @​@​ -1847\,7 +1848\,22 @​@​   if (sbrked_remains)   add_to_chain((void*)(last_sbrk_top - sbrked_remains)\,   sbrked_remains\, 0); +#if 1 +#if !defined(atarist) && !defined(__MINT__) /* on the atari we dont have to worry about this */ +# ifndef I286 /* The sbrk(0) call on the I286 always returns the next segment */ + /* WANTED_ALIGNMENT may be more than NEEDED_ALIGNMENT\, but this may + improve performance of memory access. */ + if (PTR2UV(cp) & (WANTED_ALIGNMENT - 1)) { /* Not aligned. */ + slack = WANTED_ALIGNMENT - (PTR2UV(cp) & (WANTED_ALIGNMENT - 1)); + } +# endif +#endif /* !atarist && !MINT */ + if (slack \< require) { + add_to_chain((void*)cp + slack\, require - slack\, 0); + }
+#else   add_to_chain((void*)cp\, require\, 0); +#endif   sbrk_goodness -= SBRK_FAILURE_PRICE;   sbrked_remains = 0;   last_sbrk_top = 0;

---

Service Quality Matters. Test the performance and quality of your VoIP or IP video service at​: http​://www.TestYourVoIP.com http​://www.TestYourIPVideo.com

[p5pRT]

From p5p@spam.wizbit.be

On Thu Feb 14 08​:03​:37 2008\, mnoyes@​brixnet.com wrote​:

Below is a diff to malloc.c\, in use in perl 5.6.1 and 5.8.8 that fixes a memory management bug in getpages_adjacent() that was encountered when running perl in a multi-threaded environment on Solaris. This change is essentially a replication of code from getpages(). The #if 1 conditional is provided solely to highlight the change.

Thank you for your bug report and your patch.

Would it be possible to supply some sample code which shows the problem? (so that maybe a test can be added for it)

Kind regards\,

Bram

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From mnoyes@brixnet.com

Bram\, I would like to be able to supply a test case\, but this bug is not easy to replicate\, as it depends on a bunch of enviornmental factors. I will try to describe the scenario it which it occurs\, in case that helps.

We would occasionally encounter cases when a customer would attempt to use our product and the perl code would crash with a SEGV or Bus Error repeatedly\, until the shell environment was changed somewhat. We eventually determined that the following sequence of events were occurring.

During program initialization\, the perl interpreter calls Perl_malloc for an SV which will later grow. Next\, a perl module is loaded that has a DLL with initialization code. That initialization code calls the libc malloc() or other code which causes an sbrk() to occur where the brk increase is not a multiple of NEEDED_ALIGNMENT. After the DLL is loaded\, the perl interpreter attempts to do an sv_grow on the allocation\, and since the allocation being grown is the most recent allocation (last_op)\, Perl_realloc will call getpages_adjacent to attempt to extend the allocation cheaply. In getpages_adjacent()\, we find that there are not enough left-over bytes from the prior sbrk to satisfy the re-allocation\, so sbrk is called again to attempt to extend the prior allocation. However\, because the loaded dll called sbrk()\, the program brk had changed and so Perl_realloc attempts to return the sbrk'd memory to the memory pool. Because the DLL did not sbrk an integral multiple of NEEDED_ALIGNMENT\, the new memory is not NEEDED_ALIGNMENT-aligned.

I don't remember exactly what happens from there\, but I believe that the chained memory gets used and then freed\, at which point the TWOK_MASKED macro finds the wrong "start of chunk" and reports wrong chunk size for the chunk and it's downhill from there.

So how do I know that this change fixed the problem without a reproducible test case? In addition to code inspection\, I temporarily added some debug logging to note when the corrected code path was encountered to be sure that the corrected code was being used. The corrected version has been in use for months without a recurrence of the problem.

-----Original Message----- From​: Bram via RT [mailto​:perlbug-followup@​perl.org] Sent​: Sunday\, June 29\, 2008 4​:42 AM To​: Noyes\, Mark Subject​: [perl #50856] Bug fix in malloc.c

On Thu Feb 14 08​:03​:37 2008\, mnoyes@​brixnet.com wrote​:

Below is a diff to malloc.c\, in use in perl 5.6.1 and 5.8.8 that fixes a memory management bug in getpages_adjacent() that was encountered when running perl in a multi-threaded environment on Solaris. This change is essentially a replication of code from getpages(). The #if 1 conditional is provided solely to highlight the change.

Thank you for your bug report and your patch.

Would it be possible to supply some sample code which shows the problem? (so that maybe a test can be added for it)

Kind regards\,

Bram

[p5pRT]

From david@landgren.net

Noyes\, Mark wrote​:

Bram\, I would like to be able to supply a test case\, but this bug is not easy to replicate\, as it depends on a bunch of enviornmental factors. I will try to describe the scenario it which it occurs\, in case that helps.

We would occasionally encounter cases when a customer would attempt to use our product and the perl code would crash with a SEGV or Bus Error repeatedly\, until the shell environment was changed somewhat. We eventually determined that the following sequence of events were occurring.

During program initialization\, the perl interpreter calls Perl_malloc for an SV which will later grow. Next\, a perl module is loaded that has a DLL with initialization code. That initialization code calls the libc malloc() or other code which causes an sbrk() to occur where the brk increase is not a multiple of NEEDED_ALIGNMENT.

Mixing mallocs sounds like a recipe for disaster.

David

[p5pRT]

@dcollinsn - Status changed from 'open' to 'stalled'