Overflow cygthreads (those which use simplestub) don't set notify_detached event which may cause timer_delete to hung

Overflow cygthreads (those which use simplestub) don't set notify_detached event which may cause timer_delete to hung

[Rafal Zwierz]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

Hi,

I think that this is a problem in cygthreads, but since I have been
looking at cygwin for less than two weeks I might as well be quite
mistaken.

The problem can be reproduced with Cygwin 1.7.9-1 and also with
today's checkout of the code.

To repro run this program form the attachment (compiled using: g++ main.cc)

One should observe the program hanging when deleting timer. On my
computer it usually is timer 31, but depending on race conditions you
might get a different one. If you don't get the problem then try
increasing TIMERS.

After spending long hours looking at cygwinthread.cc code I have come
up with the following patch to fix the problem. I believe that the
solution should be bullet-proof also if someone terminates the thread
(thread_terminate()) or calls detach(), but since it was the first
time I looked at the cygwin code I might as well be wrong.


* cygthread.cc (cygthread::simplestub): Notify that the thread has
detached also in freerange thread case.


Any comments are most welcome,

Best wishes,
Rafal
P.S. Please note that another (completely separate) problem with
freerange threads leaking memory in auto_release case exists. I will
create another post with info about that.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 624 bytes --]

Index: src/winsup/cygwin/cygthread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/cygthread.cc,v
retrieving revision 1.85
diff -u -p -r1.85 cygthread.cc
--- src/winsup/cygwin/cygthread.cc	30 Jul 2011 20:50:23 -0000	1.85
+++ src/winsup/cygwin/cygthread.cc	25 Aug 2011 18:44:32 -0000
@@ -136,7 +136,11 @@ cygthread::simplestub (VOID *arg)
   cygthread *info = (cygthread *) arg;
   _my_tls._ctinfo = info;
   info->stack_ptr = &arg;
+  HANDLE notify = info->notify_detached;
   info->callfunc (true);
+  if (notify) 
+     SetEvent(notify);
+    
   return 0;
 }
 

[-- Attachment #3: main.cc --]
[-- Type: application/octet-stream, Size: 1176 bytes --]

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <time.h>

#define TIMERS 40

int main()
{
   sigevent_t event;
   memset(&event, 0, sizeof(event));

   timer_t timerid[TIMERS];
   memset(timerid, 0, sizeof(timerid));

   struct itimerspec value;
   memset(&value, 0, sizeof(value)); 

   int i = 0;

   for (i = 0; i < TIMERS; i++)
   {
      event.sigev_notify = SIGEV_NONE;
      int rc = timer_create(CLOCK_REALTIME, &event, &timerid[i]);
      if (0 != rc) 
      {
         printf("Couldn't create timer %d\n", i);
         return 1;
      }
   }

   for (i = 0; i < TIMERS; i++) 
   {
      value.it_value.tv_nsec = 320000000; // 320 [ms]
      int rc = timer_settime(timerid[i], 0, &value, NULL);
      if (0 != rc) 
      {
         printf("Couldn't set timer %d\n", i);
         return 1;
      }
   }

   sleep(1); // give all the timers a chance to fire

   for (i = 0; i < TIMERS; i++) 
   {
      printf("Destroying timer %d...", i); fflush(stdout);
      int rc = timer_delete(timerid[i]);
      printf("Done\n");
      if (0 != rc) 
      {
         printf("Couldn't delete timer %d\n", i);
         return 1;
      }
   }

   return 0;
}

Re: Overflow cygthreads (those which use simplestub) don't set notify_detached event which may cause timer_delete to hung

[Christopher Faylor]

On Thu, Aug 25, 2011 at 08:06:08PM +0100, Rafal Zwierz wrote:
>* cygthread.cc (cygthread::simplestub): Notify that the thread has
>detached also in freerange thread case.

Looks good.  I'll check this in.  Thanks.

cgf