Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#8836 closed bug (invalid)

pthread_join on a detached pthread crashes

Reported by: 0x0dev Owned by: nobody
Priority: normal Milestone: R1
Component: System/POSIX Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

pthread_join should return EINVAL if the thread cannot be joined. - http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_join.html

A thread in a detached state cannot be joined - http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_attr_setdetachstate.html

It would appear that the correct behavior would be to modify the pthread_join implementation to return EINVAL if the caller attempts to join a detached thread.

Attached is an excerpt from a POSIX compatibility test that covers this specific functionality. (2-1.c). This test crashes on rev 44457, producing the error in backtrack.txt.

Attached is also a patch to amend pthread_join's behavior.

Attachments (5)

2-1.c (1.9 KB ) - added by 0x0dev 12 years ago.
backtrace.txt (1.4 KB ) - added by 0x0dev 12 years ago.
patch.txt (510 bytes ) - added by 0x0dev 12 years ago.
0002-calling-pthread_join-on-a-detached-thread-should-ret.patch (863 bytes ) - added by 0x0dev 12 years ago.
0001-calling-pthread_join-on-a-detached-thread-should-ret.patch (864 bytes ) - added by 0x0dev 12 years ago.

Download all attachments as: .zip

Change History (13)

by 0x0dev, 12 years ago

Attachment: 2-1.c added

by 0x0dev, 12 years ago

Attachment: backtrace.txt added

by 0x0dev, 12 years ago

Attachment: patch.txt added

comment:1 by 0x0dev, 12 years ago

patch: 01

comment:2 by 0x0dev, 12 years ago

I added a git format-patch style patch.

comment:3 by axeld, 12 years ago

Thanks! However, you are actually looking at a severely outdated page; issue 2 is 15 years old. The current one is issue 7, see: http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_join.html

While it should not crash, at least it's not required to return EINVAL any more. We could do this, of course. Does your patch also fix the crash?

comment:4 by bonefish, 12 years ago

Resolution: invalid
Status: newclosed

The specs are pretty explicit about calling pthread_join() on detached threads:

"The behavior is undefined if the value specified by the thread argument to pthread_join() does not refer to a joinable thread."

So, crashing is absolutely acceptable behavior. I also don't see how it can be avoided without leaking memory (or larger changes). The memory for the pthread handle is freed when a detached thread terminates, so any pthread function (including pthread_join()) using it afterwards will access invalid memory. Or IOW code that does anything with the handle of a detached pthread is broken.

Closing the ticket as invalid.

PS: Regarding your patch, please peruse our coding style guidelines. Your patch contains three violations (space after "if", testing "&" results with "=="/"!=", double semicolon).

comment:5 by 0x0dev, 12 years ago

patch: 10

comment:6 by 0x0dev, 12 years ago

patch: 01

comment:7 by 0x0dev, 12 years ago

In regards to axeld's question, yes this does prevent it from crashing.

I respect your decision to close this issue. In case you change your mind, I've attached an updated patch that addressing the coding style violations.

comment:8 by leavengood, 12 years ago

As far as style of your latest patch, it isn't quite right. The space goes after the if but before the parenthesis, see the line right above your added one.

As for the substance of the patch, it seems from Ingo that a detached thread passed in to pthread_join may no longer be valid, so I'm surprised that it works to check for the flags. In your tests that may only work because the memory used for the thread has not yet been written over, but that is certainly not always going to be the case. The fact that the free throws an error without your patch does point to this being the case.

Note: See TracTickets for help on using tickets.