This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: fix possible segfault creating detached thread


On Jul 31 15:17, Mike Gorse wrote:
> This patch fixes a seg fault when a thread is created in a detached state 
> and terminates the first time it is scheduled.  pthread::create (the 
> four-parameter version) calls the three-parameter pthread::create function 
> which unlocks the mutex, allowing the called thread to be scheduled, then 
> exits at which point the outer create function calls is_good_objectg(), 
> but this causes a core dump if pthread::exit() has already been called and 
> deleted the pthread object.

Thanks for the patch.  First, please let me point you to
http://cygwin.com/contrib.html.  The important information here is that
you'll need to fill out a copyright assignment form and snail mail it
to Red Hat if you want to get in patches.  The only exception are 
insignificant patches in terms of changed lines of code.  The usual rule of
thumb here is not more than 10 lines.  Well, your patch only changes
roughly 12 lines, so I'd let slip it in.

However, there are three tiny problems:

> 2005-07-31 Michael Gorse <mgorse@alum.wpi.edu>
> 
>         * thread.cc (pthread::create): Make bool.
>         * thread.cc (pthread_null::create): Ditto.

Wrong ChangeLog entry format.  Don't repeat the name of the file for each
change.  Compare with the current ChangeLog file.  And don't add an empty
line for each changed file, unless the changes are unrelated.

>         * thread.h: Ditto.
> 
>         * pthread.cc (pthread_create): Check return of inner create rather
>         than calling is_good_object().

This entry definitely doesn't match your below patch.  Is there really
any change in pthread.cc and did you forget to send it or is that rather
a typo and you meant to note the change to pthread::create(3 args) in
thread.cc?

> Index: thread.cc
> ===================================================================
> RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
> retrieving revision 1.190
> diff -u -p -r1.190 thread.cc
> --- thread.cc	6 Jul 2005 20:05:03 -0000	1.190
> +++ thread.cc	31 Jul 2005 02:13:14 -0000
> @@ -491,13 +491,15 @@ pthread::precreate (pthread_attr *newatt
>      magic = 0;
>  }
> 
> -void
> +bool
>  pthread::create (void *(*func) (void *), pthread_attr *newattr,
>  		 void *threadarg)
>  {
> +  bool retval;
> +
>    precreate (newattr);
>    if (!magic)
> -    return;
> +    return false;
> 
>    function = func;
>    arg = threadarg;
> @@ -517,7 +519,9 @@ pthread::create (void *(*func) (void *),
>        while (!cygtls)
>  	low_priority_sleep (0);
>      }
> +  retval =magic;

Please add a space after the '='.  Looks good otherwise.


Thanks in advance,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          mailto:cygwin@cygwin.com
Red Hat, Inc.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]