Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix silent hang when custom debug_server is not found #3756

Closed
wants to merge 1 commit into from

Conversation

nchevsky
Copy link

@nchevsky nchevsky commented Dec 5, 2020

With debug_tool = custom and debug_server set to an invalid path,
pio debug would hang indefinitely, not surface any errors, and ignore
SIGINT (i.e. it couldn't even be killed with Ctrl+C).

This change attaches an errback to the GDBClient::spawn() call in
platformio/commands/debug/command.py::cli(). The errback catches exception
DebugInvalidOptionsError asynchronously raised by DebugServer::spawn()
when server_executable cannot be found. This exception was being suppressed
because (a) there was no errback and (b) as a result of being raised from
a Deferred, the exception couldn't bubble up into the main try...except
handler in platformio/__main__.py.

The new errback stores the asynchronously caught exception and stops the
reactor. When the reactor.run() call in cli() then returns, we retrieve
the stored exception and raise it synchronously, allowing it to be caught by
the main try...except handler in platformio/__main__.py which gracefully
prints the error message and terminates the script with an error exit code.

# pylint: disable=no-member
GDBClient(project_dir, __unprocessed, debug_options, env_options) \
.spawn(ide_data["gdb_path"], ide_data["prog_path"]) \
.addErrback(handleFailure)
Copy link
Author

Choose a reason for hiding this comment

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

This new errback catches exception DebugInvalidOptionsError asynchronously raised by DebugServer::spawn()
when server_executable cannot be found. The exception was being suppressed because (a) there was no errback and (b) as a result of being raised from a Deferred, the exception couldn't bubble up into the main try...except handler in platformio/__main__.py.


# pylint: disable=import-outside-toplevel
from platformio.commands.debug.process.client import reactor
reactor.callWhenRunning(reactor.stop)
Copy link
Author

Choose a reason for hiding this comment

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

Stopping the reactor fixes the hang, allowing cli() to terminate and return control to platformio/__main__.py which will now gracefully print the exception and exit with the appropriate error code.

return True

def handleFailure(failure):
handleFailure.exception = failure.value
Copy link
Author

Choose a reason for hiding this comment

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

We store the asynchronous exception so that it can be retrieved synchronously in cli() once the call to reactor.run() returns (after the reactor is stopped by handleFailure()).


signal.signal(signal.SIGINT, lambda *args, **kwargs: None)
reactor.run()

if handleFailure.exception:
raise handleFailure.exception
Copy link
Author

Choose a reason for hiding this comment

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

By synchronously raising the asynchronously caught exception here, we allow it to be handled by the main try...except block in platformio/__main__.py which will now gracefully print the exception and exit with the appropriate error code.

@ivankravets
Copy link
Member

Thanks for the PRs! We made huge refactoring of PlatformIO Unified Debugger. See https://github.com/platformio/platformio-core/blob/develop/HISTORY.rst

Please re-retest the latest development version via pio upgrade --dev.

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.

2 participants