Try to log exceptions in IoThread#14899
Conversation
| if self.exit: | ||
| break | ||
| except Exception: | ||
| log.critical("Exception in IoThread function", exc_info=True, stack_info=True) |
There was a problem hiding this comment.
Using exc_info=True and stack_info=True) seems unusual. What is expected?
There was a problem hiding this comment.
Well, it should log the exception as well as a strack trace.
There was a problem hiding this comment.
Actually, good catch. I want to log not only a stack trace from the IoThread, but dump the stack of all threads, like the watchdog does.
| from inspect import ismethod | ||
| from buildVersion import version_year | ||
| import NVDAState | ||
| from watchdog import getFormattedStacksForAllThreads |
There was a problem hiding this comment.
Since it is not used only for watchdog, would it be worth to move getFormattedStacksForAllThreads in a log-related file? E.g. logHandler.py
With watchdog.getFormattedStacksForAllThreads remaining but being deprecated.
There was a problem hiding this comment.
I think that makes sense, but then we could also extend the stack_info argument to take something to log all stacks. That might be something more suitable for a follow up.
Related to #14899, #14312, #14627 Fixes #14895 Summary of the issue: Despite several attempts to fix this, NVDA's IoThread can crash without a clear cause. Description of user facing changes Less crashes, most likely, as tests indicate that this is the case. Description of development approach As proposed by @jcsteh , rather than creating a new function pointer for every APC or completion routine call, use a single internal APC and completion routine and use an internal cache to store the python functions, not the actual APC functions.
…fix build of developer documentation. (#15616) Related to #12971 problem caused by PR #14899 Summary of the issue: When trying to build developer documentation with Sphinx, many modules failed to be parsed Description of development approach getFormattedStacksForAllThreads is moved from watchdog to the logHandler, which avoids these warnings. Appropriate backwards compatibility is added to watchdog, to make sure add-on authors are warned to use the moved function from logHandler in the future.
Link to issue number:
None
Summary of the issue:
I'm still seeing access violations in the IoThread here and there, same for @bramd. I really want to track these down! We need to handle those more gracefully anyway.
Description of user facing changes
Better logging
Description of development approach
De run block of the IOThread is now wrapped in a try/except that logs the stack. I hope the stack will give us more info about what's going wrong here.
Testing strategy:
No longer stuff like this in the logThere should no longer be stuff like this in the log:
Rather, it should be a critical error that logs the stack. This pr needs a follow up to handle the underlying issue.
Known issues with pull request:
I can only test this with a signed try build, though I'm pretty sure this can go to Alpha pretty safely.
Change log entries:
None needed as this isn't a fix, it only improves logging to track down the underlying issue.
Code Review Checklist: