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

Add threading.main_thread() function #63082

Closed
asvetlov opened this issue Aug 30, 2013 · 16 comments
Closed

Add threading.main_thread() function #63082

asvetlov opened this issue Aug 30, 2013 · 16 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@asvetlov
Copy link
Contributor

BPO 18882
Nosy @pitrou, @vstinner, @giampaolo, @tiran, @bitdancer, @asvetlov, @serhiy-storchaka
Files
  • issue18882.diff
  • issue18882-2.diff
  • issue18882-4.diff
  • 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 2013-09-04.04:08:19.640>
    created_at = <Date 2013-08-30.10:26:45.947>
    labels = ['type-feature', 'library']
    title = 'Add threading.main_thread() function'
    updated_at = <Date 2014-03-07.22:21:04.772>
    user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2014-03-07.22:21:04.772>
    actor = 'asvetlov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-09-04.04:08:19.640>
    closer = 'asvetlov'
    components = ['Library (Lib)']
    creation = <Date 2013-08-30.10:26:45.947>
    creator = 'asvetlov'
    dependencies = []
    files = ['31519', '31530', '31544']
    hgrepos = []
    issue_num = 18882
    keywords = ['patch']
    message_count = 16.0
    messages = ['196521', '196526', '196529', '196530', '196531', '196535', '196540', '196615', '196658', '196707', '196887', '196894', '196895', '196896', '212053', '212905']
    nosy_count = 8.0
    nosy_names = ['pitrou', 'vstinner', 'giampaolo.rodola', 'christian.heimes', 'r.david.murray', 'asvetlov', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18882'
    versions = ['Python 3.4']

    @asvetlov
    Copy link
    Contributor Author

    We need public API for getting main thread object.
    See also http://comments.gmane.org/gmane.comp.python.devel/141370

    @tiran
    Copy link
    Member

    tiran commented Aug 30, 2013

    The function must take care of fork() in worker threads, too. The isinstance(current_thread(), _MainThread) trick may not work.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 30, 2013

    The function must take care of fork() in worker threads, too. The
    isinstance(current_thread(), _MainThread) trick may not work.

    Well, there are two possibilities:

    • main_thread() returns the original _MainThread instance, even if it's
      dead in the child process
    • main_thread() returns the main thread of the current process

    Both are reasonable, but we must settle for one :-)

    (also, the use case of forking from a thread is really obscure, I don't
    think we should worry too much about it)

    @asvetlov
    Copy link
    Contributor Author

    Patch with code and tests is attached.
    Test fails when program forks from thread other than the main one.

    @asvetlov
    Copy link
    Contributor Author

    signal module reinitializes main_thread variable in PyOS_AfterFork, threading does nothing with forking.

    @asvetlov
    Copy link
    Contributor Author

    http://bugs.python.org/issue16500 is required to make work after fork from thread other than the main one.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 30, 2013

    http://bugs.python.org/issue16500 is required to make work after
    fork from thread other than the main one.

    No it isn't. Please take a look at _after_fork() in threading.py.

    @asvetlov
    Copy link
    Contributor Author

    There is updated patch.
    All tests pass.
    I've added docs for threading.main_thread() function also.

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 31, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Aug 31, 2013

    Ok, some comments about the patch (no "review" links appears so I'm gonna do it inline here):

    • the doc addition needs a "versionadded" tag

    • "The main thread is the thread that the OS creates to run application.": I would rephrase this "In normal conditions, the main thread is the thread from which the Python interpreter was started".

    • in the tests:
      + self.assertEqual(data, "Thread-1\nTrue\nTrue\n")

    Hmm, how do you know it will be called "Thread-1"?
    I would give a specific name to the Thread, so as to make the test deterministic.

    + self.assertEqual(rc, 0)

    You don't need this, it is already ensured by assert_python_ok().

    • in threading.py, why doesn't _exitfunc() reuse the _main_thread global variable, instead of taking it as a parameter?

    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Sep 1, 2013

    Uploaded new patch.

    • the doc addition needs a "versionadded" tag
      Fixed.
    • "The main thread is the thread that the OS creates to run application.": I would rephrase this "In normal conditions, the main thread is the thread from which the Python interpreter was started".
      Fixed.
    • in the tests:
    •    self.assertEqual(data, "Thread-1\\nTrue\\nTrue\\n")
      

    Hmm, how do you know it will be called "Thread-1"?
    I would give a specific name to the Thread, so as to make the test deterministic.
    In this test main thread after forking is always first thread created by python. That's why it always is called 'Thread-1'.

    •    self.assertEqual(rc, 0)
      

    You don't need this, it is already ensured by assert_python_ok().
    Fixed.

    • in threading.py, why doesn't _exitfunc() reuse the _main_thread global variable, instead of taking it as a parameter?
      Fixed.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 4, 2013

    New changeset 96e55a1a0de7 by Andrew Svetlov in branch 'default':
    Issue bpo-18882: Add threading.main_thread() function.
    http://hg.python.org/cpython/rev/96e55a1a0de7

    @asvetlov asvetlov closed this as completed Sep 4, 2013
    @pitrou
    Copy link
    Member

    pitrou commented Sep 4, 2013

    I don't think you saw my review, but could you add a docstring to the main_thread() function? Thanks!

    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Sep 4, 2013

    I did not received review email, sorry.
    Docstring is added.
    BTW 'threading' module has almost no docstrings, that's why I've added only docs at first.
    Do you think docstrings should be added to all public functions?
    Thanks.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 4, 2013

    BTW 'threading' module has almost no docstrings, that's why I've
    added only docs at first.
    Do you think docstrings should be added to all public functions?

    Well, probably, although that's another issue :)

    @bitdancer
    Copy link
    Member

    In msg196529 Antoine says there are two possible interpretations of main_thread, and we must choose one. However, the documentation does not indicate which one was chosen.

    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Mar 7, 2014

    Implementation uses the first choice:
    main_thread() returns the original _MainThread instance, even if it's dead in the child process.

    I'm sorry, would you guess desired documentation change?

    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants