Repository navigation
Harden the plugin transport, codec and provider sessions - #11
Merged
Merged
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.
Summary
A full review of the transport, codec and sessions for ways a call can hang, be lost, run twice or crash a process. Bots on engine plugins were failing with
No answer to 'engine.dispatch' within 120000ms. The host side of that (the app was frozen without its foreground service) is fixed in MessengerBotR#275. This PR fixes what the contract itself did wrong. Every change has a test.Waiting on things that cannot answer
TransportClosedException:PluginTransport.sendthrows it when the other side is gone for good, andRpcPeerthen closes and fails everything pending. Before, a dead process looked like a full Binder buffer: the frame was retried out of band, and pending requests waited for their timeouts (up to 120 s). Sendingcloseorengine.interruptto the dead side also logged errors.HostSessionTransportandPluginSessionTransporttell the two cases apart withisBinderAlive.LoopbackTransportnow throws on a closed side, as a dead Binder does.EngineEndpointanswers in-flight blockinghost.calls when interrupted, after interrupting the engine. Before, a script inmsg.replyorHttp.getignored the interrupt for up to 30 s, and the host abandoned the engine thread. Checked with javet-node on the emulator: a script blocked inHttp.geton an unreachable address stopped 5 ms after the interrupt.Work the other side gave up on
engine.dispatch,provider.callandhost.callnow carry the time the asking side stops waiting (deadline, epoch ms; both processes share the device clock). Work picked up after it is answeredunavailableinstead of being run. This stops late replies to old messages after a freeze, andbot.sendrunning twice when a script retries a call that timed out behind a slow one. Requests without a deadline, from older builds, never expire.Provider sessions
provider.startcarries a generation and emits carry it back.RemoteProviderdrops emits from an earlier start: before, an emit racing a restart, or sent while a slow handler held up STOP, reached the new start with new tokens. Emits without a generation, from older plugins, are accepted.unavailable, instead of leaving the host to time out. It refuses new calls instead of running them with empty options. Options are now read when a call runs, not when it arrives, so a restart queued ahead of it applies.stop()failure is reported instead of discarded.nothingIsEmittedOnceStopped, which could not fail before, now does for such a provider (LateEmitterTest).ProviderModule.callAsync/handlecatchErrors such asTODO()in-process too, as the remote path already did. A stage completed with null, or a throw while answering, is answered instead of losing the reply.projectis not a string is dropped, not broadcast to every project.Codec and schema
MAX_OFFLOADS); the rest stay inline and the frame goes out of band as a whole. Before, a frame with 65 large byte values lost its first region to the receiver's eviction and was dropped. The receiver now keeps up to 256 regions waiting.Type.checkbuilds the path of a value only for an error. It was concatenated for every item, which was quadratic on large, deeply keyed values. Long keys are shortened in messages.?.asLongOrNullno longer turns 2^63 intoLong.MAX_VALUE. The protocol and module versions are range-checked instead of truncated to Int.Android service
IPluginServiceBinder object, whose one-way calls are delivered one at a time. Frames are now handled on each session's own thread, so a 60 MB frame or a slow handler in one project no longer holds up the others. Regions are still stored before the frame that uses them.PluginService.closereturns at once and closes the endpoint in the background, since closing runs plugin code (an engine's interrupt).send,sendSharedandclosedid not check the caller.RuntimeExceptionthrown while opening is wrapped, so it reaches the host instead of killing the plugin process.EngineEndpointandProviderEndpointtakeonProtocolError, andPluginServicelogs lost frames, such as answers that could not be sent.Compatibility
deadline,generation). Older hosts and plugins ignore them, and their absence is accepted. No protocol or Binder version change, and no AIDL change.ProviderEndpoint'sfactorymoved to the last parameter, after the newonProtocolError, so the usualProviderEndpoint(transport) { provider }still compiles. A positionalProviderEndpoint(transport, factory)must namefactory.TypegainedcheckAtandType.Path;checkis unchanged.unavailable.Testing
./gradlew :contract:test :android:testDebugUnitTest :tck:test :android:assembleRelease: 95 tests, all pass. The contract suite passed 8 runs in a row.Type.checkspeed,ErrorincallAsync.TransportClosedException; late regions are not kept; 300 large payloads in one frame; session ownership;Errorwhile opening.LateEmitterTest.msgbot-testemulator, and the plugin and engine device tests in MessengerBotR'sapp(14 classes): 41/41;The submodule bumps in MessengerBotR, NodeJSPlugin and PluginFixtures follow after this is merged.