Skip to content
This repository has been archived by the owner on Apr 6, 2021. It is now read-only.

GDB netproxy restructured as a shim #40

Merged
merged 20 commits into from
Aug 12, 2016
Merged

GDB netproxy restructured as a shim #40

merged 20 commits into from
Aug 12, 2016

Conversation

danallan
Copy link
Contributor

@danallan danallan commented Jul 29, 2016

This PR makes a major change to the GDB plugin's architecture by removing its netproxy in favor of a shim. It is changed in the following ways:

  • rather than have a runner start gdbserver while the netproxy starts GDB (the latter of which must wait for compilation before connecting to gdbserver), the shim is responsible for managing both processes. This eliminates a number of complex synchronization and process management issues.
  • this eliminates the need for a netproxy altogether, and the debugger plugin therefore communicates with the shim directly via a socket as supported by support direct connection to debugger via socket, retrying until connected #38.
  • the shim is bundled with the plugin (as the netproxy was), but on load the gdbdebugger plugin writes the shim to the workspace disk so that it is always available for a runner. The gdbdebugger is only registered as a debugger if the shim is written to disk successfully.
  • the runners that rely on GDB as a debugger therefore start the shim directly, and not gdb or gdbserver (see Update GDB runners to use new shim architecture c9.ide.run#16).

Given the change, the shim now acts as a standalone nodejs script, and accepts arguments to alter its configuration. The parsing/handling of these arguments in combination with shifting management of both gdb client and server process in the shim represent a good chunk of the line changes on this PR.

I also snuck in more generalized signal handling (in addition to just SIGSEGV) and a couple of minor other fixes to catch some common problems.

Ultimately, this should address a number of issues with regard to the GDB plugin's reliability, including both #13 and #15.

Important note:

  1. This PR depends entirely on acceptance of support direct connection to debugger via socket, retrying until connected #38 first.
  2. Once accepted, this PR should be deployed at the same time as Update GDB runners to use new shim architecture c9.ide.run#16 to ensure consistent access to the GDB debugger.

debug.registerDebugger(TYPE, plugin);

// writeFile root is workspace directory, unless given ~
var shimPath = "~/bin/c9gdbshim.js";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am looking for a way to make writefile call on demand, and only when missing
would it be ok to write this to ~/.c9/bin/c9gdbshim-<md5>.js instead, or is this supposed to be used from other places too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MD5 or other versioning would be fine, so long as we have a symlink or some other mechanism that always points to the latest revision from a known path, e.g., ~/.c9/bin/c9gdbshim.js is a link for ~/.c9/bin/c9gdbhim-<md5>.js?

@nightwing
Copy link
Member

This is absolutely awesome!

There are several small issues i need to resolve before releasing this, but they are mostly minor tweaks

  • remove hardcoded /home/ubuntu paths to allow this to work on ssh workspaces
  • show error when gdb or gdbserver are missing.
  • test on windows (optional)
  • find a way to not upload shim every time ide is loaded (nice).
  • find a way to update old runners, that are modified by users, or show a clear warning that debugging won't work.

@danallan
Copy link
Contributor Author

danallan commented Aug 3, 2016

Thanks, @nightwing! Nice list of finishing touches, too.
By the way, I had to make an additional PR just now (89b26d3) as I realized I was needlessly unlinking the socketfile on shim exit (and occasionally interfered with the shim if a second process happened to use the socket while the first was exiting). Just thought I'd point it out in case you were making tweaks on a copy of the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants