Mailing-List: contact cygwin-developers-help AT sourceware DOT cygnus DOT com; run by ezmlm
List-Unsubscribe: <mailto:cygwin-developers-unsubscribe-archive-cygwin-developers=delorie DOT com AT sourceware DOT cygnus DOT com>
List-Subscribe: <mailto:cygwin-developers-subscribe AT sourceware DOT cygnus DOT com>
List-Archive: <http://sourceware.cygnus.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT sourceware DOT cygnus DOT com>
List-Help: <mailto:cygwin-developers-help AT sourceware DOT cygnus DOT com>, <http://sourceware.cygnus.com/ml/#faqs>
Sender: cygwin-developers-owner AT sourceware DOT cygnus DOT com
Delivered-To: mailing list cygwin-developers AT sourceware DOT cygnus DOT com
Message-Id: <199909170112.UAA12971@mercury.xraylith.wisc.edu>
To: Chris Faylor <cgf AT cygnus DOT com>
cc: cygwin-developers AT sourceware DOT cygnus DOT com
Subject: [take 2] Re: (patch) makethread stdcall/cdecl confusion 
In-Reply-To: Your message of "Thu, 16 Sep 1999 22:02:17 EDT."
             <19990916220217 DOT A5792 AT cygnus DOT com> 
Date: Thu, 16 Sep 1999 20:12:47 -0500
From: Mumit Khan <khan AT xraylith DOT wisc DOT EDU>

Chris Faylor <cgf AT cygnus DOT com> writes:
> 
> Ah.  I see where you were coming from, then.  The synchronization is
> there specifically to work around this particular problem.
> 
> I don't see any reason to avoid using the stack since the
> synchronization is there to ensure that it's ok for the first thread to
> return (guess how long it took me to realize that I needed this).  I'd
> assumed that it is considerably less expensive to copy a few elements
> from the stack than it is to malloc something.  Maybe this is moot if we
> can actually eliminate the CreateEvent/SetEvent/WaitForSingleObject but
> your patch doesn't do that.  It does potentially use up a block of
> memory in the heap which could be in use for a long period of time.
> 
> Maybe what is really needed here are some comments...

My oversight. Trivially fixed by going back to structure copy and 
free'ing the passed in pointer right away. Patch below.

Frankly, I don't quite understand why the synchronization (which is
always more expensive not doing it ;-) is needed. The part that I
have not looked at in detail is what may happen in sig_send if the
signal thread hasn't gotten there quite yet. With the change below,
either way will be safe at least from the parameter lifetime issue.

Thu Sep 16 20:04:16 1999  Mumit Khan  <khan AT xraylith DOT wisc DOT edu>

	* debug.cc (stdlib.h): Unconditionally include. 
	(thread_stub): Deallocate passed parameter.
	(makethread): Don't pass stack data to thread start routine.

--- debug.cc.~1	Thu Sep 16 12:14:44 1999
+++ debug.cc	Thu Sep 16 20:03:33 1999
@@ -9,6 +9,7 @@ details. */
 #define NO_DEBUG_DEFINES
 #include "winsup.h"
 #include "exceptions.h"
+#include <stdlib.h>
 
 #undef lock
 #undef unlock
@@ -86,6 +87,8 @@ thread_stub (VOID *arg)
   exception_list except_entry;
   thread_start info = *((thread_start *) arg);
 
+  free (arg);
+
   /* marco AT ddi DOT nl: Needed for the reent's  of this local dll thread
      I assume that the local threads are using the reent structure of
      the main thread
@@ -109,11 +112,18 @@ makethread (DWORD (*start) (void *), LPV
   DWORD tid;
   HANDLE h;
   SECURITY_ATTRIBUTES *sa;
-  thread_start info;
+  // This is deallocated by thread_stub.
+  thread_start *info = (thread_start *) malloc (sizeof (thread_start));
+
+  if (! info)
+    {
+      debug_printf ("malloc failed, %E");
+      return INVALID_HANDLE_VALUE;
+    }
 
-  info.func = start;
-  info.arg = param;
-  info.sync = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
+  info->func = start;
+  info->arg = param;
+  info->sync = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
 
   if (*name != '+')
     sa = &sec_none_nih;
@@ -123,12 +133,12 @@ makethread (DWORD (*start) (void *), LPV
       sa = &sec_none;
     }
 
-  if ((h = CreateThread (sa, 0, thread_stub, (VOID *)&info, flags, &tid)))
+  if ((h = CreateThread (sa, 0, thread_stub, (VOID *)info, flags, &tid)))
     {
       regthread (name, tid);
-      WaitForSingleObject (info.sync, INFINITE);
+      WaitForSingleObject (info->sync, INFINITE);
     }
-  CloseHandle (info.sync);
+  CloseHandle (info->sync);
 
   return h;
 }
@@ -164,7 +174,6 @@ threadname (DWORD tid, int lockit)
 }
 
 #ifdef DEBUGGING
-#include <stdlib.h>
 
 typedef struct _h
   {

Regards,
Mumit