Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Birne94 mannequin opened this issue Mar 20, 2017 · 9 comments
Closed

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

Birne94 mannequin opened this issue Mar 20, 2017 · 9 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@Birne94
Copy link
Mannequin

Birne94 mannequin commented Mar 20, 2017

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
    Copy link
    Mannequin Author

    Birne94 mannequin commented Mar 20, 2017

    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 Birne94 mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Mar 20, 2017
    @Birne94
    Copy link
    Mannequin Author

    Birne94 mannequin commented Mar 20, 2017

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

    @Birne94 Birne94 mannequin added the 3.7 (EOL) end of life label Mar 20, 2017
    @methane
    Copy link
    Member

    methane commented Mar 20, 2017

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

    @Birne94
    Copy link
    Mannequin Author

    Birne94 mannequin commented Mar 20, 2017

    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
    Copy link
    Member

    methane commented Mar 21, 2017

    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
    Copy link
    Mannequin Author

    Birne94 mannequin commented Mar 21, 2017

    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
    Copy link
    Member

    methane commented Mar 21, 2017

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

    @Birne94 Birne94 mannequin closed this as completed Mar 21, 2017
    @methane
    Copy link
    Member

    methane commented Mar 24, 2017

    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
    Copy link
    Member

    methane commented Mar 24, 2017

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant