Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

get_keyboard_codes() can throw a RuntimeError if a global variable is created while it is running #248

Closed
adamnovak opened this issue Jan 27, 2023 · 4 comments

Comments

@adamnovak
Copy link

In the get_keyboard_codes() function, blessed iterates over globals().items():

https://github.com/jquast/blessed/blob/1.19.1/blessed/keyboard.py#L114

If another Python thread exists, it can create (or destroy?) a global variable while Blessed is iterating. This can cause an exception, like this one that occurred in my project:

Traceback (most recent call last):
  File "/builds/databiosphere/toil/src/toil/test/sort/sortTest.py", line 217, in testFileSingleNonCaching
    self._toilSort(jobStoreLocator=self._getTestJobStorePath(), batchSystem='single_machine',
  File "/builds/databiosphere/toil/src/toil/test/sort/sortTest.py", line 164, in _toilSort
    with runMain(options):
  File "/usr/lib/python3.9/contextlib.py", line 119, in __enter__
    return next(self.gen)
  File "/builds/databiosphere/toil/src/toil/test/sort/sortTest.py", line 59, in runMain
    main(options)
  File "/builds/databiosphere/toil/src/toil/test/sort/sort.py", line 254, in main
    sortedFileID = workflow.restart()
  File "/builds/databiosphere/toil/src/toil/common.py", line 1065, in restart
    return self._runMainLoop(rootJobDescription)
  File "/builds/databiosphere/toil/src/toil/common.py", line 1468, in _runMainLoop
    return Leader(config=self.config,
  File "/builds/databiosphere/toil/src/toil/leader.py", line 269, in run
    with enlighten.get_manager(stream=sys.stderr, enabled=not self.config.disableProgress) as manager:
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/enlighten/manager.py", line 55, in get_manager
    return Manager(stream=stream, counter_class=counter_class, **kwargs)
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/enlighten/_manager.py", line 69, in __init__
    super(Manager, self).__init__(**kwargs)
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/enlighten/_basemanager.py", line 74, in __init__
    self.term = Terminal(stream=self.stream, kind=kind, force_styling=bool(kind))
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/blessed/terminal.py", line 208, in __init__
    self.__init__keycodes()
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/blessed/terminal.py", line 312, in __init__keycodes
    self._keycodes = get_keyboard_codes()
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/blessed/keyboard.py", line 114, in get_keyboard_codes
    keycodes.update((name, value) for name, value in globals().items() if name.startswith('KEY_'))
  File "/builds/databiosphere/toil/venv/lib/python3.9/site-packages/blessed/keyboard.py", line 114, in <genexpr>
    keycodes.update((name, value) for name, value in globals().items() if name.startswith('KEY_'))
RuntimeError: dictionary changed size during iteration

Either Blessed needs to make a snapshot of globals() and then loop over it, or get_keyboard_codes() needs to catch this error and retry its loop over globals().items() repeatedly until it gets through without a concurrent modification from another thread.

@avylove
Copy link
Collaborator

avylove commented Jan 27, 2023

I wonder if globals().copy().items() would be sufficient. I would think the GIL would block during the copy operation.

@avylove
Copy link
Collaborator

avylove commented Jan 29, 2023

@adamnovak I couldn't find a quick way to replicate the behavior to test. Can you see if adding copy() resolves it for your usecase?

@adamnovak
Copy link
Author

I don't have a real reproduction for this; I've only ever seen it once and a reliable reproduction would involve writing a bunch of code to make a bunch of globals in a bunch of threads.

I think adding a copy() call would be the right way to address the issue; the dict documentation doesn't say that copy() can raise exceptions, or that it internally does any detectable "iteration".

@avylove avylove mentioned this issue Feb 3, 2023
@avylove
Copy link
Collaborator

avylove commented Feb 4, 2023

Included copy() in 1.20.0. Hopefully that takes care of it, but please let us know if you see any more issues.

@avylove avylove closed this as completed Feb 4, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants