Skip to content

Harden Ghidra remote UI lifecycle#528

Open
amattas wants to merge 3 commits into
binsync:mainfrom
amattas:pr/ghidra-remote-ui-lifecycle
Open

Harden Ghidra remote UI lifecycle#528
amattas wants to merge 3 commits into
binsync:mainfrom
amattas:pr/ghidra-remote-ui-lifecycle

Conversation

@amattas

@amattas amattas commented Jun 15, 2026

Copy link
Copy Markdown

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:

  • The child UI process could connect by autodiscovery instead of the exact server socket started by the Ghidra-side wrapper.
  • The wrapper did not reliably retain and clean up the child UI process.
  • Child process stdout/stderr were discarded, so UI-side tracebacks disappeared and Ghidra appeared to crash without a useful error.
  • Closing the Ghidra BinSync UI only disconnected the client path; it did not ask the embedded remote server to stop cleanly.
  • Once declib marks GUI Ghidra as requiring main-thread dispatch, the BinSync Ghidra wrapper must drain the server's main-thread dispatch loop or remote backend calls will block.

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:

  • Keep a handle to the child UI process and terminate/kill it during wrapper shutdown.
  • Pass the exact Unix socket URL to the child process via BINSYNC_GHIDRA_SERVER_URL.
  • Teach the child UI process to use that explicit server URL instead of relying on discovery.
  • Write child UI stdout/stderr to BINSYNC_GHIDRA_UI_LOG_PATH, defaulting to /tmp/binsync-ghidra-ui.log, so crashes leave a traceback.
  • On Ghidra UI close, stop BinSync worker routines and call shutdown_server() when the remote interface supports it.
  • If the declib server reports requires_main_thread, call wait_for_shutdown() so Ghidra backend work is dispatched on the creating/main thread.
  • Add tests for process handle retention, explicit server URL propagation, child log routing, main-thread wait behavior, UI close server shutdown, and wrapper shutdown cleanup.

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:

  • mark GUI Ghidra as requiring main-thread server dispatch, and
  • send the real shutdown_server request from DecompilerClient.shutdown_server().

Verification

  • conda run -n ghidra python -m pytest tests -q --ignore=tests/test_angr_gui.py -> 22 passed, 1 warning
  • conda run -n ghidra python -m compileall -q binsync tests
  • git diff --check

Notes

This branch intentionally combines the earlier Ghidra UI shutdown, explicit startup socket, and latest lifecycle/logging fixes. Opening those as separate PRs against upstream main would produce overlapping diffs and ordering conflicts; this single PR is the clean review unit for the Ghidra remote UI lifecycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant