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

In utils/subproc.py: Change threading.Thread monitor_thread method isAlive to is_alive #588

Merged
merged 2 commits into from Nov 13, 2020

Conversation

us3rnotfound
Copy link
Contributor

Under Python3, when I tried the command
pybombs recipes add-defaults
I got an error raised in /pybombs/utils/subproc.py:

File "/usr/local/lib/python3.9/site-packages/pybombs/utils/subproc.py", line 243, in monitor_process
    while monitor_thread.isAlive:
AttributeError: 'Thread' object has no attribute 'isAlive'

The simple fix was to change the method called isAlive to is_alive, as that was changed in python3. The command to add the default recipes worked after this fix.

…ead.is_alive, so pybombs add recipes <...> will work under python3 (and python2)
@jkbecker
Copy link
Contributor

jkbecker commented Oct 7, 2020

Good find, but I'm pretty sure we'll need to test for is_alive() (with parentheses). Testing for is_alive without parenthesis will always cast to a boolean True if the method exists, i.e. it will (unwantedly) always report that a thread is alive, such as here:

>>> from threading import Thread
>>> t = Thread()
>>> t.is_alive
<bound method Thread.is_alive of <Thread(Thread-1, initial)>>
>>> print("alive") if t.is_alive else print("not alive")
alive
>>> # note that the thread was not actually alive here, because it was never started
>>>
>>> print("alive") if t.is_alive() else print("not alive")
not alive
>>> # this is correct.

@argilo I'm not familiar enough with the codebase but I don't recall running into this while building the python3-based pybombs docker containers. Do you know why that may be? May have to go back in and check the code that's actually running there.

@jkbecker
Copy link
Contributor

jkbecker commented Oct 9, 2020

Looking into it, this must have been a bug even in python2. Python2's .isAlive() is also a method, not a property, so testing for it must have always resolved to a boolean True if used as shown above.

It is correct that .isAlive() was deprecated in favor of .is_alive() in Python 3 (.is_alive() already existed in Python 2.7 though, so it's the right variant to use anyway).

@willcode
Copy link
Member

is_alive() works, and is necessary for Fedora 33 (Python 3.9).

Copy link
Contributor

@jkbecker jkbecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with parentheses added

@jkbecker jkbecker merged commit 09afa1e into gnuradio:master Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants