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

Jump to bottom

"threading._DummyThread" redefines "is_alive" but forgets "isAlive" #79464

Closed
d-maurer mannequin opened this issue Nov 20, 2018 · 27 comments
Closed

"threading._DummyThread" redefines "is_alive" but forgets "isAlive" #79464

d-maurer mannequin opened this issue Nov 20, 2018 · 27 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@d-maurer
Copy link
Mannequin

d-maurer mannequin commented Nov 20, 2018

BPO 35283
Nosy @brettcannon, @d-maurer, @vstinner, @asvetlov, @pmp-p, @serhiy-storchaka, @corona10
PRs
  • bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11454
  • bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11454
  • bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11454
  • [3.7] bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11459
  • [3.7] bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11459
  • [3.7] bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11459
  • bpo-35283: Update the docstring of threading.Thread.join method #11596
  • bpo-35283: Update the docstring of threading.Thread.join method #11596
  • bpo-35283: Update the docstring of threading.Thread.join method #11596
  • [3.7] bpo-35283: Add deprecation warning for Thread.isAlive (GH-11454) #11604
  • [3.7] bpo-35283: Add deprecation warning for Thread.isAlive (GH-11454) #11604
  • [3.7] bpo-35283: Add deprecation warning for Thread.isAlive (GH-11454) #11604
    		•

    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 2019-01-18.14:17:47.552>
created_at = <Date 2018-11-20.22:16:29.238>
labels = ['3.7', '3.8', 'type-bug', 'library']
title = '"threading._DummyThread" redefines "is_alive" but forgets "isAlive"'
updated_at = <Date 2019-01-18.14:17:47.547>
user = 'https://github.com/d-maurer'

    bugs.python.org fields:

    activity = <Date 2019-01-18.14:17:47.547>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2019-01-18.14:17:47.552>
closer = 'asvetlov'
components = ['Library (Lib)']
creation = <Date 2018-11-20.22:16:29.238>
creator = 'dmaurer'
dependencies = []
files = []
hgrepos = []
issue_num = 35283
keywords = ['patch']
message_count = 27.0
messages = ['330158', '330216', '330219', '330237', '330242', '330244', '330245', '330247', '330248', '330277', '330278', '330279', '330280', '330293', '330472', '330917', '330922', '333156', '333170', '333854', '333856', '333911', '333930', '333951', '333954', '333955', '333966']
nosy_count = 7.0
nosy_names = ['brett.cannon', 'dmaurer', 'vstinner', 'asvetlov', 'pmpp', 'serhiy.storchaka', 'corona10']
pr_nums = ['11454', '11454', '11454', '11459', '11459', '11459', '11596', '11596', '11596', '11604', '11604', '11604']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35283'
versions = ['Python 3.7', 'Python 3.8']
    @d-maurer
    Copy link
    Mannequin Author

    d-maurer mannequin commented Nov 20, 2018

    In module "threading", class "Thread" defines "is_alive" and defines "isAlive = is_alive". The derived class "_DummyThread" redefines "is_alive" but forgets to update the "isAlive" alias. As a consequence, calling "_DummyThread.isAlive" leads to an "AssertionErrror".

    The "isAlive" method is mentioned in the docstring of "Thread.join".

    @d-maurer d-maurer mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels Nov 20, 2018
    @brettcannon
    Copy link
    Member

    Care to open a PR to fix this?

    @brettcannon brettcannon added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Nov 21, 2018
    @d-maurer
    Copy link
    Mannequin Author

    d-maurer mannequin commented Nov 21, 2018

    Care to open a PR to fix this?

    I am not familiar with the "github" workflow. The fix is so trivial (add the "isAlive = is_alive" alias to "threading._DummyThread" and replace "isAlive" by "is_alive" in the docstring of "threading.Thread.join" that it does not seem worth to learn the "github" workflow just for this fix.

    @corona10
    Copy link
    Member

    Hi, Can I work with this issue if no one works on it?

    @serhiy-storchaka
    Copy link
    Member

    isAlive() is the part of the old API. It is not even documented in Python 3, and can be removed in future. It is better to remove it from docstrings.

    @serhiy-storchaka
    Copy link
    Member

    And since threads support no longer optional, are there reasons to keep _DummyThread?

    @pitrou
    Copy link
    Member

    pitrou commented Nov 22, 2018

    If it's not already deprecated, I'd say deprecate it first.

    @corona10
    Copy link
    Member

    @serhiy.storchaka @pitrou
    Then should we add the deprecated message
    for isAlive of class "Thread" first?

    @asvetlov
    Copy link
    Contributor

    The only _DummyThread usage is threading.current_thread():

    If the caller's thread of control was not created through the threading
    module, a dummy thread object with limited functionality is returned.

    @brettcannon
    Copy link
    Member

    I guess the question is whether any other Python implementation is threadless? E.g. is MicroPython? If it even has threads then I agree about deprecating the module.

    But if MicroPython does support threads we should keep the module. That would mean updating all references of isAlive() to is_alive() and adding the name alias since that name aliasing still exists in 'master' right now (probably for Python 2 porting support):

    cpython/Lib/threading.py

    Line 1094 in 3bb183d

    isAlive = is_alive

    @dmaurer totally understand about time restraints, but do realize even "trivial" fixes like this would still take at least an hour to do a proper job so there's no guarantee someone will get around to fixing this. But if you do change your mind and want to give it a try, then https://devguide.python.org/ is there to help.

    @pmp-p
    Copy link
    Mannequin

    pmp-p mannequin commented Nov 22, 2018

    I guess the question is whether any other Python implementation is threadless?

    emscripten python ( cpython on asm.js or webassembly ) is threadless

    @pmp-p
    Copy link
    Mannequin

    pmp-p mannequin commented Nov 22, 2018

    about micropython, only unix port have thread basic implementation and garbage collector messes with EINTR actually so it is not very useable.

    unix port is only one on many, and is the less interesting port apart from running quick simulations.

    @asvetlov
    Copy link
    Contributor

    Well, to satisfy everybody we need to:

    1. Implement isAlive for dummy thread
    2. Add a deprecation warning for both Thread.isAlive and _DummyThread.isAlive
    3. Remove isAlive in future Python release (3.8 for the deprecation, 3.8+2 for removal).
    4. Backporting the property down to 3.7 and 3.6 doesn't hurt, let's do it

    Did I miss something?

    @corona10
    Copy link
    Member

    @asvetlov

    I agree with asvetlov :)

    @brettcannon
    Copy link
    Member

    I think the only thing missing from your list, Andrew, is updating docstrings and such to mention is_alive() instead of isAlive().

    @corona10
    Copy link
    Member

    corona10 commented Dec 3, 2018

    Hi, I am going to solve this issue through below process.

    1. Implement isAlive for dummy thread
    2. Updating docstrings to recommend use is_alive() instead of isAlive().
    3. Add a deprecation warning for both Thread.isAlive and _DummyThread.isAlive
    4. Remove isAlive in future Python release (3.8 for the deprecation, 3.8+2 for removal).
    5. Backporting the property down to 3.7 and 3.6 doesn't hurt, let's do it

    Is it okay?

    @d-maurer
    Copy link
    Mannequin Author

    d-maurer mannequin commented Dec 3, 2018

    Hi, I am going to solve this issue through below process.
    ...
    Is it okay?

    For me, this would be perfect.

    @vstinner
    Copy link
    Member

    vstinner commented Jan 7, 2019

    I suggest to start with:

    • Updating docstrings to recommend use is_alive() instead of isAlive(), and add a deprecation warning for Thread.isAlive

    We may apply such changes to Python 3.7, maybe also emit a PendingDeprecationWarning.

    @corona10
    Copy link
    Member

    corona10 commented Jan 7, 2019

    @vstinner
    Thanks, I've submitted PR 11459 for 3.7 which emit PendingDeprecationWarning.

    @vstinner
    Copy link
    Member

    New changeset 89669ff by Victor Stinner (Dong-hee Na) in branch 'master':
    bpo-35283: Add deprecation warning for Thread.isAlive (GH-11454)
    89669ff

    @vstinner
    Copy link
    Member

    Ok, master now emits a deprecation warning. Is it worth it to emit a pending deprecation warning in Python 3.7?

    @asvetlov
    Copy link
    Contributor

    I think yes.
    People will be notified about depreciation earlier, even after 3.8 release not everybody switches to a new version fast.
    For example, I still use 3.6 for my job now (but we are planning to switch to 3.7 in a month or two).
    Adding PendingDeprecationWarning to 3.7 is safe, I expect a very few code uses .isAlive() on Python 3.

    @corona10
    Copy link
    Member

    Great!
    Should we notify this deprecation also on Doc/whatsnew/3.7.rst?

    @asvetlov
    Copy link
    Contributor

    Not sure. IMHO it is not a *notable* change worth to be mentioned in https://docs.python.org/3/whatsnew/3.7.html#notable-changes-in-python-3-7-2

    @corona10
    Copy link
    Member

    I've upload PR 11604
    Thanks always

    @vstinner
    Copy link
    Member

    New changeset 36d9e9a by Victor Stinner (Dong-hee Na) in branch 'master':
    bpo-35283: Update the docstring of threading.Thread.join method (GH-11596)
    36d9e9a

    @vstinner
    Copy link
    Member

    New changeset c2647f2 by Victor Stinner (Dong-hee Na) in branch '3.7':
    bpo-35283: Add pending deprecation warning for Thread.isAlive (GH-11604)
    c2647f2

    @asvetlov asvetlov added 3.7 (EOL) end of life 3.8 only security fixes labels Jan 18, 2019
    @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
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    6 participants
    @brettcannon @vstinner @asvetlov @pitrou @serhiy-storchaka @corona10