Repository navigation
fix(panda): stop deadlocking on the GIL and stalling control with get_log - #65
Merged
Merged
Conversation
…_log stop_controller() joined the control thread while holding the GIL. The control thread takes the GIL to log, for instance when its loop ends with an error, so if that happened during the stop neither thread could proceed. Found on an FER: a 60 s hold ended with the motors released and the script hanging, with the main thread in std::thread::join and the control thread in take_gil. stopController() now releases the GIL around the join when it holds it, which also covers the destructor. What ended that loop was get_log(): it copied the whole log under the mutex the control thread takes for every state, so reading a long log stalled the 1 kHz loop until the robot aborted. The log now has its own mutex, which _setState only try-locks; a sample arriving during a read is not logged. Reproduced against the fake robot in the universal libfranka fork, which streams protocol 5 state: with a reflex arriving while stop_controller() waits, 1.1.0 hangs and this returns in 0.5 s; reading 17,500 samples during control left a 23.8 ms gap between commands in 1.1.0 and 3.6 ms with this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs in released 1.1.0, found together on an FER. A script held position for 60 s with logging enabled, then called
get_log()andstop_controller(). The motors released and the script hung for good.stop_controller()deadlocks. It joins the control thread while holding the GIL, and the control thread takes the GIL to log, for example when its loop ends with an error. If that happens during the stop, the main thread sits instd::thread::joinand the control thread intake_gil, as a backtrace of the hung process showed. ThePandadestructor takes the same path.get_log()stalls the 1 kHz loop. It copies the whole log under the same mutex the control thread takes for every state. With a 60 s log, the copy took long enough for the robot to abort the motion, which is what ended the loop above.Changes
stopController()releases the GIL around the join when it holds it, which covers both the binding and the destructor.get_log,enable_logginganddisable_loggingtake it;_setStatein the control loop only try-locks it. A sample that arrives during a read isn't logged, instead of the loop waiting.Test plan
There's no automated test: panda-py's CI builds can't run a control loop without a robot. Both bugs were reproduced against the fake robot in the universal libfranka fork, which streams protocol 5 state (
test/universal/fake_robot.pyonJeanElsner/libfranka@universal), with panda-py built on that fork:stop_controller()waits (--ignore-finish --reflex-after 1.5)stop_controller()returns after 0.5 sget_log()reads 17,500 samplesget_log()andstop_controller(), the sequence that hung on 1.1.0.get_log()read 60,004 samples in 216 ms with the control loop running throughout, andstop_controller()returned after 0.20 s.Found in passing, not addressed here
When a robot stops streaming state mid-motion, libfranka's UDP receive times out and shuts down the TCP socket. Its cleanup then appears to hit the network again, and an uncaught
Poco::Net::NetExceptionescapes the control thread and terminates the process. That's upstream libfranka code, and it only arises when the robot goes silent altogether, as on network loss.