Uh oh!
There was an error while loading. Please reload this page.
fix(remoteSession): re-assert kept-alive markers before applying states - #64
Open
alesgenova wants to merge 2 commits into
Open
fix(remoteSession): re-assert kept-alive markers before applying states#64alesgenova wants to merge 2 commits into
alesgenova wants to merge 2 commits into
Conversation
vtkObjectManager::UpdateObjectsFromStates only deserializes fetched states under roots carrying "vtk-object-manager-kept-alive", but any local serialization — e.g. `get` on a live object, which re-serializes its whole reference graph — rebuilds stored states WITHOUT the marker (https://gitlab.kitware.com/vtk/vtk/-/work_items/20099). One `get` on an object referencing a render window (an interactor style, a camera) permanently detaches that window's subtree from server updates: new states fetch and register but never apply, and only a page reload recovers. Track the ids the server flagged in their fetched payloads and re-stamp them via a partial registerState (which merges) right before applying states. Ids are dropped when the server ignores them, and re-stamping is restricted to ids in the current update's dependency report so one render window's update never touches another's roots.
Comment on lines
+425
to
+433
| const reportedIds = new Set(serverStatus.ids.map(([id]) => Number(id))); | ||
| this.keptAliveStateIds.forEach((id) => { | ||
| if (reportedIds.has(id)) { | ||
| this.#native.registerState({ | ||
| Id: id, | ||
| "vtk-object-manager-kept-alive": true, | ||
| }); | ||
| } | ||
| }); |
Member
There was a problem hiding this comment.
not entirely convinced this is necessary because this updateAsync method fetches the states from the server before coming here and the server would've inserted the kept-alive marker.
The null-state branch deleted this.stateMTimes[vtkId], but vtkId is fetchStateAsync's parameter and was never in scope here — a null state in a batch response threw a ReferenceError and killed the whole update. The response is positionally aligned with the requested ids, so drop the ledger entry for stateIds[i] instead.
jspanchu
commented
Aug 20, 2026
Member
your fix(remoteSession): correct out-of-scope id in fetchBatchAsync commit is straightforward. can you create separate PR with it? I would be happy to merge that one. |
jspanchu
commented
Aug 28, 2026
Member
Seb added the |
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 freeto 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.
vtkObjectManager::UpdateObjectsFromStates only deserializes fetched states under roots carrying "vtk-object-manager-kept-alive", but any local serialization — e.g.
geton a live object, which re-serializes its whole reference graph — rebuilds stored states WITHOUT the marker (https://gitlab.kitware.com/vtk/vtk/-/work_items/20099). Onegeton an object referencing a render window (an interactor style, a camera) permanently detaches that window's subtree from server updates: new states fetch and register but never apply, and only a page reload recovers.Track the ids the server flagged in their fetched payloads and re-stamp them via a partial registerState (which merges) right before applying states. Ids are dropped when the server ignores them, and re-stamping is restricted to ids in the current update's dependency report so one render window's update never touches another's roots.