nvda
e0ba12cf - Follow up of #14312: ensure IO completion routines and waitable timers are safe (#14627)

Commit
2 years ago
Follow up of #14312: ensure IO completion routines and waitable timers are safe (#14627) Follow up of #14312 Summary of the issue: In #14312, we decoupled the background I/O thread from the braille module. However, the thread was still bound to IoBase in it's _initialRead method unless you'd override it. While I hoped the changes in #14312 would decrease crashing of braille, this was indeed the cause. However, crashes are still occurring here and there. I'm pretty sure I know the cause lies in the IO done completion routines that are still executed on the IO thread without ensuring that the instances of the routines still exist. I've seen this causing several access violation errors. Furthermore, #14312 saved bound methods in a dictionary on the iOThread. While this shouldn't be a big problem, this could cause potential issues with garbage collection (i.e. instances being kept alife forever because the APC of an instance was never called and it would be stuck in the cached apc dictionary). Description of user facing changes Hopefully, more stability. Description of development approach Added the ioThread as a constructor method to IoBase and derivatives as an optional argument. If not provided (default), the default IoThread is used. It will be added as a weak reference on the IoBase instance. Thereby it is no longer necessary to subclass IoBase or a derivative to override _initialRead to use another thread. Added IoThread.setWaitableTimer and IoThread.getCompletionRoutine, also ensuring that the function pointers are available when the background thread tries to call them. The function wrapper keeps a weak reference to the method it wraps. Thereby it ensures that a method of a died instance is no longer executed. IoThread.queueAsApc still stores a strong reference to the wrapped method on its APC, to keep backwards compatibility. This will be changed to weak references starting in NVDA 2024.1. API deprecation logic is in place. Removed the pre_IoThreadStop extension point. It was added in 2023.1 but never implemented and announced, so I think it is safe to do so.
Author
Parents
Loading