allwpilib
allwpilib copied to clipboard
RobotPy aborts in nt::impl::EntryNotifierThread during interpreter shutdown when using SendableBuilder
Reference (and some stack traces): https://github.com/robotpy/robotpy-wpiutil/pull/40
Actually using the builder causes ntcore to abort() on robot exit ... because the listener gets deregistered on python shutdown, which causes the std::function wrappers for get/set to be destroyed, which tries to take the GIL, which aborts because you can't take the GIL on another python thread on shutdown.
I don't currently implement a poller thread (I know, I know... ), but even if I did, if C++ code creates a normal listener, my reading of the callback manager code is that that will bypass my poller if I did create one? I guess maybe I could implement my own IEntryNotifier and force ntcore to use it, but I'm not actually sure that's possible either? Open to suggestions @PeterJohnson .
This blocks users from creating their own wpi::Sendable objects in RobotPy.
I suspect this is a case of us holding it wrong. There's a wpi::SendableHelper
which should do the right thing.
Any update on this? If the problem isn't on the RobotPy side, do we know more specifically where the problem is on the WPILib side?
There are also many changes here with NT4.
I would like to change the C++ side to allow me to hook thread creation to fix some of these sorts of issues. I'm still working through wpimath however, so hopefully I'll have some concrete suggestions soon.
The alternative might be to have a different std::function wrapper that allows the python callbacks to be registered globally and then removed when the interpreter shuts down, bypassing some of these deregistration issues. I did this for something different and it was effective. There might be a way to do it in pybind11 also.
In NT4, we're expecting most use cases to use polling, rather than async notifications called from a separate thread, but the latter still does exist as a feature. The thread is per-instance though so if you destroy the instance, the thread is joined before DestroyInstance() returns (and thus any callbacks won't get called after that point). Could you add a shutdown hook that destroys any instances that have been created?