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

make threading fork-safe #32958

Closed
cgw mannequin opened this issue Aug 19, 2000 · 8 comments
Closed

make threading fork-safe #32958

cgw mannequin opened this issue Aug 19, 2000 · 8 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@cgw
Copy link
Mannequin

cgw mannequin commented Aug 19, 2000

BPO 401226
Nosy @gvanrossum, @tim-one
Files
  • None: None
  • 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 = 'https://github.com/gvanrossum'
    closed_at = <Date 2000-08-27.17:37:19.000>
    created_at = <Date 2000-08-19.04:30:35.000>
    labels = ['interpreter-core']
    title = 'make threading fork-safe'
    updated_at = <Date 2000-08-27.17:37:19.000>
    user = 'https://bugs.python.org/cgw'

    bugs.python.org fields:

    activity = <Date 2000-08-27.17:37:19.000>
    actor = 'gvanrossum'
    assignee = 'gvanrossum'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2000-08-19.04:30:35.000>
    creator = 'cgw'
    dependencies = []
    files = ['2726']
    hgrepos = []
    issue_num = 401226
    keywords = ['patch']
    message_count = 8.0
    messages = ['33938', '33939', '33940', '33941', '33942', '33943', '33944', '33945']
    nosy_count = 3.0
    nosy_names = ['gvanrossum', 'tim.peters', 'cgw']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue401226'
    versions = []

    @cgw
    Copy link
    Mannequin Author

    cgw mannequin commented Aug 19, 2000

    No description provided.

    @cgw cgw mannequin closed this as completed Aug 19, 2000
    @cgw cgw mannequin assigned gvanrossum Aug 19, 2000
    @cgw cgw mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 19, 2000
    @cgw cgw mannequin closed this as completed Aug 19, 2000
    @cgw cgw mannequin assigned gvanrossum Aug 19, 2000
    @cgw cgw mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 19, 2000
    @cgw
    Copy link
    Mannequin Author

    cgw mannequin commented Aug 19, 2000

    See http://www.lambdacs.com/newsgroup/FAQ.html#Q120
    and "man pthread_atfork" for background.

    I don't use a pthread_atfork handler here - the explicit
    approach is more portable. However calling fork() from
    C extensions could still cause trouble.

    This patch causes the child to create a new interpreter
    lock after doing a fork. It would be nice to deallocate
    the old lock with PyThread_free_lock, but this does some
    unwanted error-checking in addition to deallocating the
    lock. So I waste a little memory instead. To really do
    this cleanly one could add a new PyThread_reset_lock function to all the thread_*.h files, and use that instead.

    @tim-one
    Copy link
    Member

    tim-one commented Aug 20, 2000

    Assigned to me. I'll discuss it with Guido too. So far 2e've gotten 3 reports from people who don't see failures in assorted test cases anymore, and no reports of remaining failures.

    @tim-one
    Copy link
    Member

    tim-one commented Aug 24, 2000

    Accepted, and assigned to Jeremy for checkin.
    We may (or may not) want to bulletproof more locks for 2.0b1, but this has certainly helped so far and done no harm. Jeremy, I don't feel comfortable trying to check in the change myself, as I can't run test_fork1 (or any other fork test) on Windows.

    1 similar comment
    @tim-one
    Copy link
    Member

    tim-one commented Aug 24, 2000

    Accepted, and assigned to Jeremy for checkin.
    We may (or may not) want to bulletproof more locks for 2.0b1, but this has certainly helped so far and done no harm. Jeremy, I don't feel comfortable trying to check in the change myself, as I can't run test_fork1 (or any other fork test) on Windows.

    @gvanrossum
    Copy link
    Member

    Don't check this in yet!

    Tim & I had a brainwave. We can share a single mutex for all locks and grab it around a fork, and the lock reinitialization won't be needed.

    (We think.)

    @tim-one
    Copy link
    Member

    tim-one commented Aug 25, 2000

    Now that I've had a chance to drive recklessly, suck down a Coke, and chain-smoke two butts in peace <wink>, I'm certain that the "one mutex" idea will work, and satisfied that the extra contention it may cause is pthread's own damn fault for presuming to kill threads in the child while they're in an unknown state. Full mutex ahead!

    @gvanrossum
    Copy link
    Member

    The great idea Tim & I had didn't solve the problem!

    The problem has a *parent* spinning. We suspect now that it is a LinuxThreads or kernel bug, since there's otherwise no way that what happens in the child can affect the parent. (Charles' fix only adds code that runs in the child.)

    So I've checked in Charles' patch and leave the thread code alone.

    Do remember that a long chain of forks will leak a bit -- I see no way around that.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants