Skip to content

fix(remoteSession): re-assert kept-alive markers before applying states #64

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.

Sign up for GitHub

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

Open
alesgenova wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(remoteSession): re-assert kept-alive markers before applying states #64

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be62122
fix(remoteSession): re-assert kept-alive markers before applying states
alesgenova Aug 14, 2026
bab21e1
fix(remoteSession): correct out-of-scope id in fetchBatchAsync
alesgenova Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions src/remoteSession.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ export class RemoteSession {
this._pendingUpdates = new Map(); // vtkId -> queued-but-not-started run (coalescing)
this.currentMTime = 1;
this.stateMTimes = {};
// ids whose fetched state carried "vtk-object-manager-kept-alive". The
// deserializer only applies fetched states to subtrees rooted in a state
// with that marker, but any local serialization (e.g. `get` on a live
// object) rebuilds the stored state WITHOUT it (see
// https://gitlab.kitware.com/vtk/vtk/-/work_items/20099). Remember which
// ids the server flagged so updateAsync can re-stamp them before applying
// states; otherwise one `get` on an object referencing a render window
// silently detaches that window from all future server updates.
this.keptAliveStateIds = new Set();
this.hashesMTime = {};
this.pendingArrays = {};
this.networkFetchState = null;
Expand Down Expand Up @@ -216,6 +225,9 @@ export class RemoteSession {
const state = serverState ? JSON.parse(serverState) : null;
if (state) {
this.stateMTimes[state.Id] = state.MTime;
if (state["vtk-object-manager-kept-alive"]) {
this.keptAliveStateIds.add(Number(state.Id));
}
} else {
delete this.stateMTimes[vtkId];
}
Expand Down Expand Up @@ -261,9 +273,12 @@ export class RemoteSession {
const state = states[i];
if (state) {
this.stateMTimes[state.Id] = state.MTime;
if (state["vtk-object-manager-kept-alive"]) {
this.keptAliveStateIds.add(Number(state.Id));
}
results.push(state);
} else {
delete this.stateMTimes[vtkId];
delete this.stateMTimes[stateIds[i]];
}
this.incrementProgress("state");
}
Expand Down Expand Up @@ -382,9 +397,10 @@ export class RemoteSession {
serverStatus.cameras.forEach((v) => this.cameraIds.add(Number(v)));

// Remove state that should be ignored
serverStatus.ignore_ids.forEach((vtkId) =>
this.#native.unRegisterState(vtkId),
);
serverStatus.ignore_ids.forEach((vtkId) => {
this.#native.unRegisterState(vtkId);
this.keptAliveStateIds.delete(Number(vtkId));
});

// Ensure completion of all network calls
await Promise.all(pendingWork.hashes);
Expand All @@ -400,6 +416,22 @@ export class RemoteSession {
}
}

// Re-assert the ownership markers before applying states: registerState
// merges partial states into stored ones, so this restores root status
// that a local serialization stripped since the last update (see the
// keptAliveStateIds comment in the constructor). Restrict to ids in this
// update's dependency report — other render windows' roots are not
// listed here and must not be touched from this call.
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,
});
}
});
Comment on lines +425 to +433

@jspanchu jspanchu Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Bump local mtime and process states to reflect server state
try {
this.#native.updateObjectsFromStates();
Expand Down