Rockhopper-Technologies / enlighten

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

Rapidly printing to stderr from separate process causes overwriting of progress bar #36

Closed SeanDS closed 3 years ago

SeanDS commented 3 years ago

Describe the bug When the stderr stream contains a lot of lines in rapid succession, in this case from a separate process spamming stderr, occasionally the progress bar gets overwritten by stderr lines:

terminal

I am not sure if this is really a bug, or just a fact of life due to race conditions on the same terminal using multiprocessing.

To Reproduce I adapted the demo script:

# Copyright 2019 - 2020 Avram Lubkin, All Rights Reserved

# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

"""
Demo of Enlighten's features
"""

import sys
import random
import time
from multiprocessing import Process

import enlighten

def spam():
    while True:
        print(random.random(), file=sys.stderr)

def initialize(manager, initials=15):
    """
    Simple progress bar example
    """

    # Simulated preparation
    pbar = manager.counter(total=initials, desc='Initializing:', unit='initials')
    for _ in range(initials):
        time.sleep(random.uniform(0.05, 0.25))  # Random processing time
        pbar.update()
    pbar.close()

def main():
    """
    Main function
    """

    process = Process(target=spam)

    with enlighten.get_manager() as manager:
        status = manager.status_bar(status_format=u'Enlighten{fill}Stage: {demo}{fill}{elapsed}',
                                    color='bold_underline_bright_white_on_lightslategray',
                                    justify=enlighten.Justify.CENTER, demo='Initializing',
                                    autorefresh=True, min_delta=0.5)
        docs = manager.term.link('https://python-enlighten.readthedocs.io/en/stable/examples.html',
                                 'Read the Docs')
        manager.status_bar(' More examples on %s! ' % docs, position=1, fill='-',
                           justify=enlighten.Justify.CENTER)

        process.start()
        initialize(manager, 100)
        status.update(demo='Loading')

if __name__ == '__main__':
    main()

Environment (please complete the following information):

avylove commented 3 years ago

Thanks for reporting. I'm not sure I can completely eliminate this, but I definitely think there is room for improvement. I'll work on this and let you know when I have something to test.

SeanDS commented 3 years ago

Thanks! Great little library, BTW, it works nicely for normal use cases of my application. It's only when super verbose mode gets enabled, when there's lots of log noise on stderr, that this issue pops up.

avylove commented 3 years ago

Glad you like the library! Tell your friends :-)

Fix is in master. I'll try to push a new release today or tomorrow. Please try it out.

Essentially I've consolidated calls to write to the stream. This should result in atomic behavior, but could vary by platform. If I understand correctly, on Linux, writes are guaranteed atomic-like behavior if they don't exceed the buffer size. On my Fedora 33 install, the default buffer size is 8 KiB, way larger than we need. I'm not sure what the behavior is for other OSes, but this fix should help them regardless since there will be few calls to write to the stream. Additionally, as a side effect, some unnecessary calls were removed, so there should be a small efficiency gain.

SeanDS commented 3 years ago

Please try it out.

Done - works great, at least for me on Arch!

Tell your friends :-)

I will probably in the near future use this library in a bigger project which gets used by Linux, Mac and Windows users, so if/when I do this there will be a wider pool of test platforms and I can feed back any issues encountered - so the atomic write behaviour can be tested out.

Thanks a lot for the quick fix - much appreciated!

avylove commented 3 years ago

Happy to help! Just published 1.7.1 with the fix. Please let me know if you spot any more bugs.

For your use case, you may want to consider passing threaded=True to the manager. If this is False a resize is handled when a resize signal is received, and ,if True, the resize is delayed until the next update. The default is True when multiple threads are detected, but it just occurred to me now, I should add logic to detect if multiple processes are running and apply it as a default then too. This avoid a possible reentrant call which will raise a RuntimeError.