fix: remove DataStore add-core listener on close()#1282
Merged
Conversation
DataStore registered a permanent 'add-core' listener on the CoreManager but never removed it on close(). If coreManager emitted 'add-core' after the DataStore's MultiCoreIndexer had closed (a teardown racing with a sync session or peer connection adding a core), the listener called addCore() on a closed indexer, throwing 'Cannot add core after closing' from inside a synchronous EventEmitter callback (uncaught). Keep a reference to the bound listener and remove it in close() before closing the indexer. Closes #1277
Member
Author
|
@RangerMauve thoughts on why the windows workflow is timing out? It seems like this normally completes in about 9 mins, but I've re-run it 3 times and it keeps failing here. Not sure what in this PR would change things, but maybe it's a github infra issue and these machines are just running slow today? In which case should we increase our timeout? |
Contributor
|
I think it's just that one test that's flaky. Unsure if their infra is just slower right now and its surfacing a race condition somewhere. Defs not related to this PR though. |
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
2 participants
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.
Fixes #1277.
DataStoreregistered a permanentadd-corelistener on theCoreManagerbut never removed it onclose(). IfcoreManageremittedadd-coreafter theDataStore'sMultiCoreIndexerhad closed (a teardown racing with a sync session or peer connection adding a core), the listener calledaddCore()on a closed indexer, throwingCannot add core after closingfrom inside a synchronousEventEmittercallback (uncaught).In CoMapeo mobile this latched the embedded backend into a permanent
ERRORstate, which the frontend then re-threw on every subsequent RPC send — a storm of fatal/ErrorBoundary crashes (Sentry COMAPEO-20J / 20H / 213).Changes
add-corelistener as a private field andcoreManager.off('add-core', …)it inclose()before closing the indexer.close()removes the add-core listener (listenerCountreturns to baseline).Related
addCore()a no-op when closed, so a late add-core during teardown can't throw even if a caller misses removing a listener.🤖 Generated with Claude Code