Harden Ghidra remote UI lifecycle#528
Open
amattas wants to merge 3 commits into
Open
Conversation
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.
Problem
The Ghidra integration launches a BinSync UI process that connects back to a declib server running inside Ghidra. That remote UI path had several lifecycle and diagnosability problems:
These combine into the user-visible symptom where errors in the Ghidra path can crash or strand Ghidra with little/no visible traceback.
Solution
This PR folds the related Ghidra UI lifecycle fixes into one clean branch:
BINSYNC_GHIDRA_SERVER_URL.BINSYNC_GHIDRA_UI_LOG_PATH, defaulting to/tmp/binsync-ghidra-ui.log, so crashes leave a traceback.shutdown_server()when the remote interface supports it.requires_main_thread, callwait_for_shutdown()so Ghidra backend work is dispatched on the creating/main thread.Dependency
Depends on binsync/declib#200 for the full Ghidra crash/lifecycle fix.
The BinSync side can still improve logging and process cleanup on its own, but the main-thread-dispatch and remote-server-stop pieces need declib to:
shutdown_serverrequest fromDecompilerClient.shutdown_server().Verification
conda run -n ghidra python -m pytest tests -q --ignore=tests/test_angr_gui.py->22 passed, 1 warningconda run -n ghidra python -m compileall -q binsync testsgit diff --checkNotes
This branch intentionally combines the earlier Ghidra UI shutdown, explicit startup socket, and latest lifecycle/logging fixes. Opening those as separate PRs against upstream
mainwould produce overlapping diffs and ordering conflicts; this single PR is the clean review unit for the Ghidra remote UI lifecycle.