Skip to content

Commit deca4de

Browse files
kmccarpsambsnyd
andauthored
Roll back localRefs on failed GetObject exchange (#6932)
When a GetObject exchange fails mid-serialization, the sender's localRefs retains refs assigned during the failed exchange. On subsequent exchanges, the sender sends pure references for objects the remote never received, causing "Received a reference to an object that was not previously sent". Fix by snapshotting the ref count before the exchange and rolling back any refs added during a failed exchange, on both Java and TypeScript sender sides. Fixes moderneinc/customer-requests#1977 Co-authored-by: Sam Snyder <sam@moderne.io>
1 parent cd67cfb commit deca4de

4 files changed

Lines changed: 46 additions & 6 deletions

File tree

rewrite-core/src/main/java/org/openrewrite/rpc/request/GetObject.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ protected List<RpcObjectData> handle(GetObject request) throws Exception {
8484

8585
RpcSendQueue sendQueue = new RpcSendQueue(batchSize.get(), batch::put, localRefs, request.getSourceFileType(), traceGetObject.get());
8686
TREE_TRAVERSAL_POOL.submit(() -> {
87+
// Snapshot the current ref count so we can roll back on failure.
88+
// Ref IDs are assigned sequentially as localRefs.size() + 1,
89+
// so any ref > savedRefCount was added during this exchange.
90+
int savedRefCount = localRefs.size();
8791
try {
8892
sendQueue.send(after, before, null);
8993

@@ -96,6 +100,13 @@ protected List<RpcObjectData> handle(GetObject request) throws Exception {
96100
// forces a full object sync (ADD) instead of a delta (CHANGE)
97101
// against the stale, partially-sent baseline.
98102
remoteObjects.remove(id);
103+
104+
// Roll back localRefs to remove refs assigned during this failed
105+
// exchange. Without this, subsequent exchanges would send pure
106+
// references for objects the remote never received, causing
107+
// "Received a reference to an object that was not previously sent".
108+
localRefs.values().removeIf(ref -> ref > savedRefCount);
109+
99110
PrintStream logFile = log.get();
100111
//noinspection ConstantValue
101112
if (logFile != null) {

rewrite-core/src/test/java/org/openrewrite/rpc/RewriteRpcTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,14 @@ void sendFailureCleansUpRemoteObjects() {
181181
}
182182

183183
// Step 4: verify the sender cleaned up its stale remoteObjects entry
184+
// and rolled back any refs assigned during the failed exchange
184185
assertThat(server.remoteObjects)
185186
.describedAs("Sender should remove stale remoteObjects entry after send failure")
186187
.doesNotContainKey(id);
188+
int refsAfterFailure = server.localRefs.size();
189+
assertThat(refsAfterFailure)
190+
.describedAs("Sender should roll back localRefs assigned during failed exchange")
191+
.isEqualTo(0);
187192

188193
// Step 5: put back a valid tree and retry — should succeed via full ADD
189194
PlainText fixed = original.withText("Fixed");

rewrite-javascript/rewrite/src/reference.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,19 @@ export class ReferenceMap {
8484
this.refs.set(ref, refId);
8585
this.refsById.set(refId, ref);
8686
}
87-
}
87+
88+
snapshot(): number {
89+
return this.refCount;
90+
}
91+
92+
rollbackTo(savedRefCount: number): void {
93+
for (let i = savedRefCount; i < this.refCount; i++) {
94+
const obj = this.refsById.get(i);
95+
if (obj) {
96+
this.refs.delete(obj);
97+
this.refsById.delete(i);
98+
}
99+
}
100+
this.refCount = savedRefCount;
101+
}
102+
}

rewrite-javascript/rewrite/src/rpc/request/get-object.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,20 @@ export class GetObject {
6363
const after = obj;
6464
const before = remoteObjects.get(objId);
6565

66-
allData = await new RpcSendQueue(localRefs, request.sourceFileType, trace())
67-
.generate(after, before);
68-
pendingData.set(objId, allData);
69-
70-
remoteObjects.set(objId, after);
66+
// Snapshot ref count so we can roll back on failure.
67+
// Ref IDs are assigned sequentially, so any ref >= savedRefCount
68+
// was added during this exchange.
69+
const savedRefCount = localRefs.snapshot();
70+
try {
71+
allData = await new RpcSendQueue(localRefs, request.sourceFileType, trace())
72+
.generate(after, before);
73+
pendingData.set(objId, allData);
74+
remoteObjects.set(objId, after);
75+
} catch (e) {
76+
remoteObjects.delete(objId);
77+
localRefs.rollbackTo(savedRefCount);
78+
throw e;
79+
}
7180
}
7281

7382
const batch = allData.splice(0, batchSize);

0 commit comments

Comments
 (0)