The cygheap saga

[Pierre A. Humblet]

At 10:37 PM 4/1/2004 -0500, Christopher Faylor wrote:
>On Thu, Apr 01, 2004 at 09:36:14PM -0500, Pierre A. Humblet wrote:
>>At 06:11 PM 4/1/2004 +0200, Corinna Vinschen wrote:
>>>
>>>AFAICS, we have currently three pressing problems:
>>>
>>>- People complaining about cygheap problems (Win32 error 487) but
>>>  nobody willing to debug it.
>>
>>I will describe how it works (details in how-cygheap-works),
>>and then how we might change that to improve the chance of
>>success.
>>
>>1) When starting, Cygwin tries to reserve an amount of memory Pr
>>   as large as possible with CYGHEAPSIZE >= Pr >= CYGHEAPSIZE_MIN
>>   The initial committed memory size Pc is 0.
>>2) As Cygwin operates and calls csbrk , Pc grows. Pc can grow
>>   past Pr, although that is unlikely, and in fact impossible if
>>   Pr <  CYGHEAPSIZE. csbrk calls will not fail as long as Pc <= Pr.
>>3) When starting a child, the parent creates a file mapping of size 
>>   Max(Pc, Pr), copies Pc bytes to it and passes the handle to the child.
>>4) The child tries to map the region to the same address as the parent's
>>   heap. If that works, the child is done.
>>5) Else the child tries to reserve Max(Pc, Pr) at the same address
>>   as the parent's heap.
>>6) The child then commits and copies Pc bytes from the file mapping to
>>   that region.
>>The failure occurs in step 5.
>>
>>What I don't see if why we need to reserve Max(Pc, Pr) and not simply
>>a quantity Cr with Pr >= Cr >= Pc or Pr >= Cr >= max(Pc, CYGHEAPSIZE_MIN).
>>We could even try to make Cr as large as CYGHEAPSIZE, but if the parent
>>did not succeed I would think the child won't either.
>>As an aside, I am wondering if the current step 5 can ever work (on NT)
>>if step 4 has failed. Why would 4 fail if space is available? 
>
>Let me make it really simple.
>
>Parent is reserving X amount of space.
>
>Child can't reserve that much space because something (probably a DLL)
>is blocking the reservation of the full amount of space.
>
>Before we change the design, we need to find out what is blocking the
>reservation of the memory rather than just making the child allocate less
>space than the parent.

Why not do both, given that this problem seems to pop up regularly?
At any rate I had written a patch and I tested on NT today (on a machine 
without problem). The curious can try it. It's a fairly simple patch.

Pierre
-------------- next part --------------
Index: cygheap.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygheap.cc,v
retrieving revision 1.101
diff -u -p -r1.101 cygheap.cc
--- cygheap.cc	26 Mar 2004 22:48:47 -0000	1.101
+++ cygheap.cc	2 Apr 2004 20:02:08 -0000
@@ -144,17 +144,35 @@ cygheap_fixup_in_child (bool execed)
 {
   cygheap = child_proc_info->cygheap;
   cygheap_max = child_proc_info->cygheap_max;
+  alloc_sz = child_proc_info->cygheap_alloc_sz;
   void *addr = !wincap.map_view_of_file_ex_sucks () ? cygheap : NULL;
   void *newaddr;

   newaddr = MapViewOfFileEx (child_proc_info->cygheap_h, MVMAP_OPTIONS, 0, 0, 0, addr);
-  alloc_sz = child_proc_info->cygheap_alloc_sz;
   if (newaddr != cygheap)
     {
-      if (!newaddr)
-	newaddr = MapViewOfFileEx (child_proc_info->cygheap_h, MVMAP_OPTIONS, 0, 0, 0, NULL);
       DWORD n = (DWORD) cygheap_max - (DWORD) cygheap;
-      /* Reserve cygwin heap in same spot as parent */
+      if (addr)
+        {
+	  alloc_sz = n + (128 * 1024); /* Minimum */
+
+	  if (!newaddr)
+	    newaddr = MapViewOfFileEx (child_proc_info->cygheap_h, MVMAP_OPTIONS, 0, 0, 0, NULL);
+
+	  MEMORY_BASIC_INFORMATION m;
+
+	  if (!VirtualQuery ((LPCVOID) cygheap, &m, sizeof m))
+	    debug_printf ("couldn't get memory info, %E");
+
+	  else if ((char *) m.BaseAddress + m.RegionSize >= (char *) cygheap + alloc_sz)
+	    alloc_sz = (char *) m.BaseAddress - (char *) cygheap + m.RegionSize;
+
+          system_printf ("Desired: %d Reserved: %d Committed %d",
+		         child_proc_info->cygheap_alloc_sz, alloc_sz, n);
+	}
+      if (!newaddr)
+	api_fatal ("MapViewOfFileEx failed");
+	  /* Reserve cygwin heap in same spot as parent */
       if (!VirtualAlloc (cygheap, alloc_sz, MEM_RESERVE, PAGE_NOACCESS))
 	{
 	  MEMORY_BASIC_INFORMATION m;
@@ -167,15 +185,14 @@ cygheap_fixup_in_child (bool execed)
 	  api_fatal ("m.AllocationBase %p, m.BaseAddress %p, m.RegionSize %p, m.State %p\n",
 		     m.AllocationBase, m.BaseAddress, m.RegionSize, m.State);
 	}
-
-      /* Allocate same amount of memory as parent */
+
+      /* Commit same amount of memory as parent */
       if (!VirtualAlloc (cygheap, n, MEM_COMMIT, PAGE_READWRITE))
 	api_fatal ("Couldn't allocate space for child's heap %p, size %d, %E",
 		   cygheap, n);
       memcpy (cygheap, newaddr, n);
       UnmapViewOfFile (newaddr);
     }
-
   ForceCloseHandle1 (child_proc_info->cygheap_h, passed_cygheap_h);

   cygheap_init ();