Summary: | Wrong pthread_create calls all over the place | ||
---|---|---|---|
Product: | Claws Mail (GTK 2) | Reporter: | Christian Cornelssen <ccorn> |
Component: | Other | Assignee: | users |
Status: | RESOLVED FIXED | ||
Severity: | blocker | ||
Priority: | P3 | ||
Version: | 3.2.0 | ||
Hardware: | Macintosh | ||
OS: | Mac OS X 10.2 | ||
Attachments: |
Description
Christian Cornelssen
2008-01-20 00:50:13 UTC
Created attachment 536 [details]
Patch that rewinds some of claws-mail's liberalization w.r.t. pthreads back to 2.10.0 usage for PPC-Mach (Mac OS X / Darwin)
The patch to 3.2.0 as described in the original bug report.
It would be even better to find out the underlying reason... Could you investigate? > It would be even better to find out the underlying reason... Could you
> investigate?
I have taken a closer look:
src/common/ssl.c and src/matcher.c clearly violate the prototype of pthread_create() by passing PTHREAD_CREATE_JOINABLE (#defined to be 1 in my <pthread.h>) where a pthread_attr_t* is expected. I wonder why others don't seem to get bus errors / memory protection errors from this. (Perhaps the constant is 0 on other systems, and passing NULL might be valid, although my manpage does not document this.)
I suppose one has to alloc some pthread_attr_t object, then do a pthread_attr_init() on it and add a call to pthread_attr_setdetachstate() using the desired PTHREAD_CREATE_JOINABLE.
I am wondering what the precise motivation for the glibc-related conditionals has been. Could it be that glibc somehow manages to avoid bus errors (though probably not setting the desired detachstate), and so someone just thought that only glibc was suitable for his code? If so, then the conditionals could be reduced to #if USE_PTHREAD after fixing the code.
The third occurence of the glibc-related conditional is in src/mimeview.c, but the type signature of the call to pthread_create seems ok. There is even a nice call to pthread_attr_setdetachstate() using TRUE as argument, which equals PTHREAD_CREATE_JOINABLE, I suppose. (Note that the alternative PTHREAD_CREATE_DETACHED (== 2) is also nonzero, so the use of TRUE is somewhat cryptic and questionable.)
Haven't tried to clean this up yet.
Created attachment 538 [details]
repairs broken pthread calls (particularly useful for Darwin)
I have tried to clean up the pthread mess. Offered for review here, as I am not familiar with claws-mail code. Let's call this a "plausible fix" for a bunch of obvious miscodings.
* src/common/ssl.c (SSL_connect_nb):
- Fixed the call to pthread_create() by creating and passing an appropriate pthread_attr_t object.
- The GNUTLS variant of the blocking (fallback) code did not return its result, thus reaching code that would wait for the nonexistent thread to terminate. Fixed.
* src/matcher.c (matcherprop_match_test):
- Fixed the call to pthread_create() by creating and passing an appropriate pthread_attr_t object.
* src/mimeview.c (mimeview_check_sig_in_thread):
- Use PTHREAD_CREATE_DETACHED instead of TRUE (which would mean PTHREAD_CREATE_JOINABLE at least on Darwin) in the call to pthread_attr_setdetachstate().
- check return codes of pthread_attr_init() and pthread_attr_setdetachstate().
* src/common/ssl.c (SSL_connect_nb), src/matcher.c (matcherprop_match_test), src/mimeview.c (check_signature_cb):
- Removed the glibc-related #if conditions near USE_PTHREAD, assuming that their only purpose was to specify the lucky cases where the broken code seemed to function.
I should add that patch #538 compiles and works on my powerpc-apple-darwin8.11.0. At least, there have been no crashes or hangups so far. (In reply to comment #5) >I am wondering what the precise motivation for the glibc-related conditionals >has been. Could it be that glibc somehow manages to avoid bus errors (though >probably not setting the desired detachstate), and so someone just thought that >only glibc was suitable for his code? Yes. (That was me...) Looks like this code hasn't been scrutinized enough! >I should add that patch #538 compiles and works on my >powerpc-apple-darwin8.11.0. At least, there have been no crashes or hangups so >far. Applying, thanks a bunch. Changes related to this bug have been committed. Please check latest CVS and update the bug accordingly. You can also get the patch from: http://www.colino.net/claws-mail/ 2008-01-21 [colin] 3.2.0cvs57 * AUTHORS * src/matcher.c * src/mimeview.c * src/common/ssl.c * src/gtk/authors.h Fix bug 1478, 'Wrong pthread_create calls all over the place'. Patch by Christian Cornelssen |