Rockhopper-Technologies / enlighten

Enlighten Progress Bar for Python Console Apps
https://python-enlighten.readthedocs.io
Mozilla Public License 2.0
416 stars 25 forks source link

Crash during window resizing #28

Closed lps-rocks closed 4 years ago

lps-rocks commented 4 years ago

Describe the bug When resizing the window the thread running enlighten crashes

To Reproduce Resize window while running with multiple threads

Environment (please complete the following information):

Additional context


Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_manager.py", line 275, in _resize_handler
    assert self.resize_lock
AssertionError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "./archive.py", line 633, in <module>
    status[r[0]].refresh()
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_basecounter.py", line 230, in refresh
    flush=flush, counter=self)
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_manager.py", line 472, in write
    stream.write(u'\r' + term.clear_eol + output)
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_manager.py", line 303, in _resize_handler
    cter.refresh(flush=False)
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_basecounter.py", line 230, in refresh
    flush=flush, counter=self)
  File "/usr/local/lib/python3.7/dist-packages/enlighten/_manager.py", line 472, in write
    stream.write(u'\r' + term.clear_eol + output)
RuntimeError: reentrant call inside <_io.BufferedWriter name='<stdout>'>
lps-rocks commented 4 years ago

It looks like because the resizing work is done inside the resize signal handler this is triggering the bug. Python project suggests deferring any changes to outside the signal handler.

https://bugs.python.org/issue24283

avylove commented 4 years ago

Thanks for reporting. Just a couple questions. Are you using Enlighten within multiple threads? Do you have sample code that demonstrates the behavior?

lps-rocks commented 4 years ago

@avylove - I am using it with multiple threads, the manager and all of the status bars are called and updated from the main thread, however there are some other threads that are using logging and could be writing to the enlighten logger.

I did also just realize I'm not on the latest version and it does look like there were some changes to the resizing functions between 1.6.0 and 1.6.1 - That could be playing a part. I'm going to test 1.6.1 as soon as I get my internal python mirror sync'd (for some reason it stopped syncing).

avylove commented 4 years ago

Enlighten doesn't have a logger. It's looks like the resize handler is trying to use stdout when another thread has it.

I'm not sure the latest version will make a difference if I'm understanding the issue. Essentially the resize_handler will have to only flag that a resize needs to occur and the resize would have to be delayed until the next time a write is made by the manager. That seems doable, but I'm not sure when I can get to it.

In the meantime, if you want to disable resize, you can pass no_resize to the manager.

avylove commented 4 years ago

I'll try to make the changes, but if you can send me some minimal code to demonstrate it would go a long way.

lps-rocks commented 4 years ago

I'll see if I can get some code that easily reproduces it that isn't protected by copyright. Thanks much for checking this out.

So I did try latest version, it also did not solve the issue, what I did find is that if i wrote to stdout from any other thread while resizing it would trigger this bug. If I wrote to the logger from only the main thread where enlighten was being called from, this wouldn't happen.

avylove commented 4 years ago

I was able to reproduce the issue with the following code. Now that I have a way to test, I'll try to work on a fix.

import os
import random
import signal
import sys
import threading
import time

import enlighten

RUN = True
TOTAL = 100
PID = os.getpid()

def print_hello():
    while RUN:
        print('hello')

def resize_constantly():
    while RUN:
        os.kill(PID, signal.SIGWINCH)

manager = enlighten.get_manager()
ticks = manager.counter(total=TOTAL, desc='Ticks', unit='ticks')
assert manager.process_exit

t1 = threading.Thread(target=print_hello)
t2 = threading.Thread(target=resize_constantly)
t1.start()
t2.start()

try:
    for num in range(TOTAL):
        time.sleep(random.uniform(0.05, 0.1))
        print(num)
        ticks.update()
except:
    RUN = False
    raise

manager.stop()
RUN = False
avylove commented 4 years ago

@lps-rocks I put a fix in 021a2561d629db014952713f8697678c10a043f7. Please try it out and confirm it resolves your issue.

avylove commented 4 years ago

Fixed in 1.62. Let me know if you have any issues.

avylove commented 4 years ago

I may have back this out in the next release because it has unintended consequences. Specifically, the output gets corrupted if something is written to the screen between the resize event and the next call of manager.write(). This can be fixed by wrapping stdout so it handles resizing events on the next write, but that could get complicated.

avylove commented 3 years ago

Didn't back out, but 1.7.0 introduces a new parameter for Manager(), threaded. If threaded is True, resizing will be deferred, otherwise it will be handled by the signal handler. threaded defaults to True if multiple signals are detected, threading.active_count() > 1, so it should be automatic if the manager is initialized after threads are created, but I suggest using enlighten.get_manager(threaded=True) to be sure.