fixing some issues with remote modules#165
Draft
mohitulm wants to merge 2 commits into
Draft
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.
Currently, in remote rpyc server-client setup, the client breaks when the server shuts down. Also the client should not be able to control deactivation of the remote server modules. The PR introduces changes to address these issues.
Description
Three related fixes for remote (rpyc) module robustness:
Fix
TypeErroron deactivation of remote modules with_threaded = True.On the client,
self._instanceis an rpyc netref, not aQObject. PySide'sstricter type check on
QMetaObject.invokeMethodrejects it. The threadedcode path also doesn't make sense for remote modules — their thread lives on
the server.
activatenow skips the threaded branch whenself.is_remote;deactivateskips it via the new local-only path (item 3 below).Reproduces with any
scanner_guitoolchain, becauseScanningProbeDummyBareis the only dummy module that sets
_threaded = True.Survive remote server shutdown. Previously, an
EOFError/OSErrorfroma dead rpyc connection propagated out of
state,is_active,is_busy,_disconnect, and_disable_state_updated, wedging the client.statereturns'DISCONNECTED'(socket errors) or'BROKEN'(othererrors) for remote modules instead of raising. Local modules re-raise as
before. Removes the previous bare
except:that also swallowedKeyboardInterrupt.is_active/is_busyroute throughstateand returnFalsesafely._disconnectand_disable_state_updatedtolerate dead connections andQt's already-deleted-object errors so cleanup runs to completion.
activateauto-reloads a remote module that's in'BROKEN'or'DISCONNECTED'state._poll_module_statelogs an error once per disconnect event.Don't drive server-side deactivation from the client. Previously,
deactivate()on a remote module calledmodule_state.deactivate()anddisconnect_modules()over rpyc, tearing down server-side FSM andconnector state. Now remote
deactivate()only cleans up locally: stopsthe poll timer, drops the proxy, emits signals. The server's module is
left untouched. Re-activating on the client fetches a fresh proxy.
Backward compatibility
'DISCONNECTED'and'BROKEN'are new state strings, only emitted forremote modules under the failure conditions above.
server-side FSM. Users who relied on this should deactivate from the
server side instead.
How Has This Been Tested?
Tested with a remote server and client config for some modules.
scanner_guiand quitting via Ctrl+C: previously crashed withTypeError; now exits cleanly.now logs the disconnect, marks modules as
'DISCONNECTED', and staysusable.
without having to restart the client.
server's module state.
Types of changes
Checklist:
/docs/changelog.md.(syntax, indentation, mutable default values, etc.).