Re: Bug in cmalloc? Was: Re: Problems with: Improvements to fork handling (2/5)

[Ryan Johnson <ryan dot johnson at cs dot utoronto dot ca>]

On 30/05/2011 2:24 AM, Christopher Faylor wrote:
> On Sun, May 29, 2011 at 12:27:45PM -0400, Christopher Faylor wrote:
> > On Sun, May 29, 2011 at 01:51:35AM -0400, Ryan Johnson wrote:
> > > So, I defined this small function:
> > >
> > > static void break_cmalloc(int depth, int maxdepth) {
> > >      void* x = cmalloc (HEAP_2_DLL, 32);
> > >      cfree(x);
> > >      if (depth<  maxdepth)
> > >          break_cmalloc(depth+1, maxdepth);
> > > }
> > >
> > > and called it during fork instead of dlls.topsort(), with maxdepth=5. No
> > > bug (as expected).
> > >
> > > Then I moved the call to cfree below the recursion, so memory gets freed
> > > in reverse order. Bang. Bash goes down and takes mintty with it after
> > > briefly flashing 'bad address.' Calling bash from cmd.exe hangs the
> > > latter so badly Windows can't kill it (I guess I'll have to reboot).
> > Thanks for the test case.  I'll investigate later today.
> As it turns out, this is not a bug in cmalloc.  fork() was not designed
> to allow calling cmalloc() after the "frok grouped" definition at the
> beginning of the function.  That records the bounds of the cygheap which
> is passed to the child.  If you call cmalloc and friends after that then
> the child process will never know that the heap has been extended.  Then
> when the heap is copied from the parent by cygheap_fixup_in_child(),
> you'll likely be missing pieces of the parent's cygheap and pieces of
> the freed pool will end up pointing into blocks of memory which are not
> properly initialized.
Ouch... and those pieces that didn't get copied are exactly the ones the 
child tries to read first when loading dlls.

Sorry for the false alarm -- I always assumed that was done after the 
call to setjmp, on the parent's side. Should have checked.

BTW, while poring over the patch I noticed a couple irregularities which 
you might want to remove:

1. The declaration for populate_all_deps in dll_init.h is never defined 
or used
2. The incrementing of the (seemingly dead) variable 'tot' ended up in 
dll_list::append and should be moved back to dll_list::alloc where it 
belongs.

Ryan