From 22a293bd51c3bb98f67d8d5c4362391b9e9e4637 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 13 Feb 2025 11:47:51 +1100 Subject: [PATCH 1/5] tests: fixed logger setting for test --- tests/QUICStream.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/QUICStream.test.ts b/tests/QUICStream.test.ts index 313a8bbf..1c3f151e 100644 --- a/tests/QUICStream.test.ts +++ b/tests/QUICStream.test.ts @@ -11,7 +11,7 @@ import * as testsUtils from './utils'; import { generateTLSConfig, sleep } from './utils'; describe(QUICStream.name, () => { - const logger = new Logger(`${QUICStream.name} Test`, LogLevel.INFO, [ + const logger = new Logger(`${QUICStream.name} Test`, LogLevel.WARN, [ new StreamHandler( formatting.format`${formatting.level}:${formatting.keys}:${formatting.msg}`, ), From b87bfb9a26a1079b359de07b696bb29db3226204 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 13 Feb 2025 11:51:30 +1100 Subject: [PATCH 2/5] fix: minor fix to `processStreams` logic --- src/QUICConnection.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/QUICConnection.ts b/src/QUICConnection.ts index 7e1008fe..446f8a19 100644 --- a/src/QUICConnection.ts +++ b/src/QUICConnection.ts @@ -1031,9 +1031,6 @@ class QUICConnection { } try { this.conn.streamSend(streamId, Buffer.alloc(0), false); - utils.never( - 'We never expect the stream to be writable if it was created during the writable iterator', - ); } catch (e) { // If we got `FinalSize` during the writable iterator then we cleaned up an errant stream if (e.message === 'FinalSize') continue; @@ -1070,6 +1067,9 @@ class QUICConnection { } utils.never(`Expected to throw "FinalSize", got ${e.message}`); } + utils.never( + 'We never expect the stream to be writable if it was created during the writable iterator', + ); } quicStream.write(); } From a8a0909720b79e23d3cbc30502c78ffc19196c78 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 13 Feb 2025 11:52:29 +1100 Subject: [PATCH 3/5] 1.3.11 --- Cargo.lock | 2 +- Cargo.toml | 2 +- package-lock.json | 14 +++++++------- package.json | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35679f1e..21c53e5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -388,7 +388,7 @@ dependencies = [ [[package]] name = "quic" -version = "1.3.10" +version = "1.3.11" dependencies = [ "boring", "napi", diff --git a/Cargo.toml b/Cargo.toml index f288d5f2..7a39e56d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "quic" -version = "1.3.10" +version = "1.3.11" authors = ["Roger Qiu "] license-file = "LICENSE" edition = "2021" diff --git a/package-lock.json b/package-lock.json index 052ba4bb..0fd0ba06 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@matrixai/quic", - "version": "1.3.10", + "version": "1.3.11", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@matrixai/quic", - "version": "1.3.10", + "version": "1.3.11", "license": "Apache-2.0", "dependencies": { "@matrixai/async-cancellable": "^1.1.1", @@ -57,11 +57,11 @@ "typescript": "^5.1.6" }, "optionalDependencies": { - "@matrixai/quic-darwin-arm64": "1.3.10", - "@matrixai/quic-darwin-universal": "1.3.10", - "@matrixai/quic-darwin-x64": "1.3.10", - "@matrixai/quic-linux-x64": "1.3.10", - "@matrixai/quic-win32-x64": "1.3.10" + "@matrixai/quic-darwin-arm64": "1.3.11", + "@matrixai/quic-darwin-universal": "1.3.11", + "@matrixai/quic-darwin-x64": "1.3.11", + "@matrixai/quic-linux-x64": "1.3.11", + "@matrixai/quic-win32-x64": "1.3.11" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index 17273598..b5b94f54 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@matrixai/quic", - "version": "1.3.10", + "version": "1.3.11", "author": "Matrix AI", "contributors": [ { @@ -48,11 +48,11 @@ "ip-num": "^1.5.0" }, "optionalDependencies": { - "@matrixai/quic-darwin-arm64": "1.3.10", - "@matrixai/quic-darwin-universal": "1.3.10", - "@matrixai/quic-darwin-x64": "1.3.10", - "@matrixai/quic-linux-x64": "1.3.10", - "@matrixai/quic-win32-x64": "1.3.10" + "@matrixai/quic-darwin-arm64": "1.3.11", + "@matrixai/quic-darwin-universal": "1.3.11", + "@matrixai/quic-darwin-x64": "1.3.11", + "@matrixai/quic-linux-x64": "1.3.11", + "@matrixai/quic-win32-x64": "1.3.11" }, "devDependencies": { "@fast-check/jest": "^1.1.0", From 6af7ca6c2a7f940b330cdbce90cbd9bca2f6cfd9 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 13 Feb 2025 15:15:50 +1100 Subject: [PATCH 4/5] fix: applying alternative fix for zombie streams by tracking used `StreamId`s --- src/QUICConnection.ts | 120 ++++++++++++++++++++++++++++-------------- 1 file changed, 81 insertions(+), 39 deletions(-) diff --git a/src/QUICConnection.ts b/src/QUICConnection.ts index 446f8a19..9136c5ce 100644 --- a/src/QUICConnection.ts +++ b/src/QUICConnection.ts @@ -104,6 +104,26 @@ class QUICConnection { */ protected streamIdServerUni: StreamId = 0b11 as StreamId; + /** + * Tracks the highest StreamId that has a created QUICStream for clientBidi + */ + protected streamIdUsedClientBidi = -1 as StreamId; + + /** + * Tracks the highest StreamId that has a created QUICStream for serverBidi + */ + protected streamIdUsedServerBidi = -1 as StreamId; + + /** + * Tracks the highest StreamId that has a created QUICStream for clientUni + */ + protected streamIdUsedClientUni = -1 as StreamId; + + /** + * Tracks the highest StreamId that has a created QUICStream for clientUni + */ + protected streamIdUsedServerUni = -1 as StreamId; + /** * Quiche connection timer. This performs time delayed state transitions. */ @@ -967,6 +987,30 @@ class QUICConnection { } } + protected isStreamUsed(streamId: StreamId): boolean { + const type = 0b11 & streamId; + switch (type) { + case 0b00: + if (streamId <= this.streamIdUsedClientBidi) return true; + this.streamIdUsedClientBidi = streamId; + return false; + case 0b01: + if (streamId <= this.streamIdUsedServerBidi) return true; + this.streamIdUsedServerBidi = streamId; + return false; + case 0b10: + if (streamId <= this.streamIdUsedClientUni) return true; + this.streamIdUsedClientUni = streamId; + return false; + case 0b11: + if (streamId <= this.streamIdUsedServerUni) return true; + this.streamIdUsedServerUni = streamId; + return false; + default: + utils.never('got an unexpected ID type'); + } + } + protected processStreams() { for (const streamId of this.conn.readable() as Iterable) { let quicStream = this.streamMap.get(streamId); @@ -985,7 +1029,9 @@ class QUICConnection { ); continue; } - + if (this.isStreamUsed(streamId)) { + utils.never('We should never repeat streamIds when creating streams'); + } quicStream = QUICStream.createQUICStream({ initiated: 'peer', streamId, @@ -1029,46 +1075,39 @@ class QUICConnection { ); continue; } - try { - this.conn.streamSend(streamId, Buffer.alloc(0), false); - } catch (e) { - // If we got `FinalSize` during the writable iterator then we cleaned up an errant stream - if (e.message === 'FinalSize') continue; - if (utils.isStreamStopped(e) !== false) { - // In this case it was a stream that was created but errored out. We want to create a new stream for this one case. - quicStream = QUICStream.createQUICStream({ - initiated: 'peer', - streamId, - config: this.config, - connection: this, - codeToReason: this.codeToReason, - reasonToCode: this.reasonToCode, - logger: this.logger.getChild(`${QUICStream.name} ${streamId}`), - }); - this.streamMap.set(quicStream.streamId, quicStream); - quicStream.addEventListener( - events.EventQUICStreamSend.name, - this.handleEventQUICStreamSend, - ); - quicStream.addEventListener( - events.EventQUICStreamDestroyed.name, - this.handleEventQUICStreamDestroyed, - { once: true }, - ); - quicStream.addEventListener( - EventAll.name, - this.handleEventQUICStream, - ); - this.dispatchEvent( - new events.EventQUICConnectionStream({ detail: quicStream }), - ); - quicStream.write(); - continue; + if (this.isStreamUsed(streamId)) { + try { + this.conn.streamSend(streamId, new Uint8Array(), false); + } catch (e) { + // Both `StreamStopped()` and `FinalSize` errors means that the stream has ended and we cleaned up state + if (utils.isStreamStopped(e) !== false) continue; + if (e.message === 'FinalSize') continue; + throw e; } - utils.never(`Expected to throw "FinalSize", got ${e.message}`); + utils.never('We never expect a duplicate stream to be readable'); } - utils.never( - 'We never expect the stream to be writable if it was created during the writable iterator', + quicStream = QUICStream.createQUICStream({ + initiated: 'peer', + streamId, + config: this.config, + connection: this, + codeToReason: this.codeToReason, + reasonToCode: this.reasonToCode, + logger: this.logger.getChild(`${QUICStream.name} ${streamId}`), + }); + this.streamMap.set(quicStream.streamId, quicStream); + quicStream.addEventListener( + events.EventQUICStreamSend.name, + this.handleEventQUICStreamSend, + ); + quicStream.addEventListener( + events.EventQUICStreamDestroyed.name, + this.handleEventQUICStreamDestroyed, + { once: true }, + ); + quicStream.addEventListener(EventAll.name, this.handleEventQUICStream); + this.dispatchEvent( + new events.EventQUICConnectionStream({ detail: quicStream }), ); } quicStream.write(); @@ -1178,6 +1217,9 @@ class QUICConnection { } else if (this.type === 'server' && type === 'uni') { streamId = this.streamIdServerUni; } + if (this.isStreamUsed(streamId!)) { + utils.never('We should never repeat streamIds when creating streams'); + } const quicStream = QUICStream.createQUICStream({ initiated: 'local', streamId: streamId!, From 267e2a187f87067ada76640f1aa9b823d7dd01a3 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Thu, 13 Feb 2025 15:16:33 +1100 Subject: [PATCH 5/5] 1.3.12 --- Cargo.lock | 2 +- Cargo.toml | 2 +- package-lock.json | 14 +++++++------- package.json | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 21c53e5e..3580ff96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -388,7 +388,7 @@ dependencies = [ [[package]] name = "quic" -version = "1.3.11" +version = "1.3.12" dependencies = [ "boring", "napi", diff --git a/Cargo.toml b/Cargo.toml index 7a39e56d..c3895ce1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "quic" -version = "1.3.11" +version = "1.3.12" authors = ["Roger Qiu "] license-file = "LICENSE" edition = "2021" diff --git a/package-lock.json b/package-lock.json index 0fd0ba06..76f2f859 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@matrixai/quic", - "version": "1.3.11", + "version": "1.3.12", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@matrixai/quic", - "version": "1.3.11", + "version": "1.3.12", "license": "Apache-2.0", "dependencies": { "@matrixai/async-cancellable": "^1.1.1", @@ -57,11 +57,11 @@ "typescript": "^5.1.6" }, "optionalDependencies": { - "@matrixai/quic-darwin-arm64": "1.3.11", - "@matrixai/quic-darwin-universal": "1.3.11", - "@matrixai/quic-darwin-x64": "1.3.11", - "@matrixai/quic-linux-x64": "1.3.11", - "@matrixai/quic-win32-x64": "1.3.11" + "@matrixai/quic-darwin-arm64": "1.3.12", + "@matrixai/quic-darwin-universal": "1.3.12", + "@matrixai/quic-darwin-x64": "1.3.12", + "@matrixai/quic-linux-x64": "1.3.12", + "@matrixai/quic-win32-x64": "1.3.12" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index b5b94f54..e12210af 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@matrixai/quic", - "version": "1.3.11", + "version": "1.3.12", "author": "Matrix AI", "contributors": [ { @@ -48,11 +48,11 @@ "ip-num": "^1.5.0" }, "optionalDependencies": { - "@matrixai/quic-darwin-arm64": "1.3.11", - "@matrixai/quic-darwin-universal": "1.3.11", - "@matrixai/quic-darwin-x64": "1.3.11", - "@matrixai/quic-linux-x64": "1.3.11", - "@matrixai/quic-win32-x64": "1.3.11" + "@matrixai/quic-darwin-arm64": "1.3.12", + "@matrixai/quic-darwin-universal": "1.3.12", + "@matrixai/quic-darwin-x64": "1.3.12", + "@matrixai/quic-linux-x64": "1.3.12", + "@matrixai/quic-win32-x64": "1.3.12" }, "devDependencies": { "@fast-check/jest": "^1.1.0",