-
-
Notifications
You must be signed in to change notification settings - Fork 795
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
Conversation
# pylint: disable=no-member | ||
GDBClient(project_dir, __unprocessed, debug_options, env_options) \ | ||
.spawn(ide_data["gdb_path"], ide_data["prog_path"]) \ | ||
.addErrback(handleFailure) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
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 |
With
debug_tool = custom
anddebug_server
set to an invalid path,pio debug
would hang indefinitely, not surface any errors, and ignoreSIGINT (i.e. it couldn't even be killed with Ctrl+C).
This change attaches an errback to the
GDBClient::spawn()
call inplatformio/commands/debug/command.py::cli()
. The errback catches exceptionDebugInvalidOptionsError
asynchronously raised byDebugServer::spawn()
when
server_executable
cannot be found. This exception was being suppressedbecause (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 incli()
then returns, we retrievethe stored exception and raise it synchronously, allowing it to be caught by
the main
try...except
handler inplatformio/__main__.py
which gracefullyprints the error message and terminates the script with an error exit code.