gnuradio / pybombs

PyBOMBS (Python Build Overlay Managed Bundle System) is the GNU Radio install management system for resolving dependencies and pulling in out-of-tree projects.
https://gnuradio.org
GNU General Public License v3.0
416 stars 189 forks source link

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

Closed us3rnotfound closed 3 years ago

us3rnotfound commented 4 years ago

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.

jkbecker commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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