Return code of pthread_* in thread_pthread.h is not used for perror

[Birne94]

BPO	29859
Nosy	@methane, @Birne94
PRs	bpo-29859: Fix error messages from return codes for pthread_* calls #741 [3.6] bpo-29859: Fix error messages from return codes for pthread_* calls #753
Files	patch.diff test.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-03-21.13:13:21.233>
created_at = <Date 2017-03-20.12:08:19.583>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'Return code of pthread_* in thread_pthread.h is not used for perror'
updated_at = <Date 2017-03-24.20:10:11.360>
user = 'https://github.com/Birne94'

bugs.python.org fields:

activity = <Date 2017-03-24.20:10:11.360>
actor = 'methane'
assignee = 'none'
closed = True
closed_date = <Date 2017-03-21.13:13:21.233>
closer = 'Birne94'
components = ['Interpreter Core']
creation = <Date 2017-03-20.12:08:19.583>
creator = 'Birne94'
dependencies = []
files = ['46746', '46749']
hgrepos = []
issue_num = 29859
keywords = ['patch']
message_count = 9.0
messages = ['289884', '289887', '289892', '289904', '289908', '289930', '289932', '290122', '290123']
nosy_count = 2.0
nosy_names = ['methane', 'Birne94']
pr_nums = ['741', '753']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29859'
versions = ['Python 3.6', 'Python 3.7']

[Birne94]

Python/thread_pthread.h:145 defines the CHECK_STATUS macro used for printing error messages in case any of the calls fail.

CHECK_STATUS uses perror for formatting an error message, which relies on the global erno being set (see man perror). Since the pthread functions return their status code instead of setting erno (which might not even work in threaded environments), no additional information is displayed. See for example produced by PyThread_release_lock:

pthread_mutex_lock[3]: Undefined error: 0
pthread_cond_signal: Undefined error: 0
pthread_mutex_unlock[3]: Undefined error: 0

The correct solution would be to use strerror(status) in order to show the proper message.

[Birne94]

I have attached a diff adding a new macro for handling pthread_* status codes. Will submit PR as soon as my CLA is approved.

[methane]

Could you give us minimum sample Python code to reproduce the error?

[Birne94]

While you might scold me for the following code, it is just some fiddling with the cpython functions from python side.

Backstory is bpo-6721 or related to it. I am working on a multi-process service which uses multiprocessing (or rather the billiard fork). While forking I run into deadlocks as described in the issue when I fork while the io lock is acquired.

In order to find a solution I experimented with the cpython module and tried to release the lock manually after a fork. The code can be found attached (tested with python 3.6/3.7 on OSX Sierra). While it does not really do what it is supposed to do, it shows the problem described in this issue (bpo-29859):

If the internal calls to the pthread_* functions fail, I will only get a generic error message:

pthread_mutex_lock[3]: Undefined error: 0
pthread_cond_signal: Undefined error: 0
pthread_mutex_unlock[3]: Undefined error: 0

In reality the produced error messages should be:

pthread_mutex_lock[3]: Invalid argument
pthread_cond_signal: Invalid argument
pthread_mutex_unlock[3]: Invalid argument

This happens because the pthread_* functions do not set the erno variable as described in the issue description.

Please note that the issue is not only related to my code, but might affect all code using locks. While I suppose there shouldn't be any errors when calling the low level functions, the attached patch/pr will make debugging a lot easier when it actually happens by displaying the correct error message.

[methane]

I don't know your patch is worth enough or not. (I dislike fprintf(stderr, ...) at least).

But my advice is stop mixing multithreading and fork (except fork+exec).
It's almost impossible.

While Python has GIL, some extension can release GIL and run any C code.
But very vary functions are not fork-safe. Even malloc and printf are unsafe.

For more information, see "rational" section in
http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_atfork.html

[Birne94]

While I agree, fprintf it not really nice, I looked through other parts of the python source where information is printed to stderr and fprintf was used there as well, so I fell back to it myself.

% grep -rnw . -e "fprintf(stderr," | wc -l 178

Using threading and multiprocessing is insane, I know that. Nevertheless the error codes returned by the pthread_* calls are processed incorrectly, so I would consider this a bug worth fixing.

[methane]

OK, perror() writes to stderr too. fair enough.

[methane]

New changeset fe30339 by INADA Naoki in branch '3.6':
bpo-29859: Fix error messages from return codes for pthread_* calls (GH-753)
fe30339

[methane]

New changeset d7fa6b2 by INADA Naoki (Daniel Birnstiel) in branch 'master':
bpo-29859: Fix error messages from return codes for pthread_* calls (GH-741)
d7fa6b2