Mail Archives: cygwin-developers/1999/09/16/22:24:40
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
- Raw text -