From a1b7179342277fa6f961dd93989aba83a4d691de Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 11:46:54 +0800 Subject: [PATCH 1/5] fix typo --- tests/unit/APITest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/APITest.ts b/tests/unit/APITest.ts index d34a07a9b16e..7c7182059bec 100644 --- a/tests/unit/APITest.ts +++ b/tests/unit/APITest.ts @@ -581,7 +581,7 @@ describe('APITests', () => { }); }); - test('Read request should not stuck when SequentialQueue is paused an resumed', async () => { + test('Read request should not stuck when SequentialQueue is paused and resumed', async () => { // Given 2 WRITE requests and 1 READ request where the first write request pauses the SequentialQueue const xhr = jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({previousUpdateID: 1}); API.write('MockWriteCommandOne' as WriteCommand, {}); From 5a4018bd9f0364a55b8cbe0c4fa25c7f0e680e63 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 11:54:39 +0800 Subject: [PATCH 2/5] add comment --- src/libs/Network/SequentialQueue.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 223966f26a6f..0a0c5281a3a9 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -198,6 +198,12 @@ function unpause() { const numberOfPersistedRequests = PersistedRequests.getAll().length || 0; Log.info(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); isQueuePaused = false; + + // A READ request will wait until all the WRITE requests are done, using isReadyPromise promise. + // When the queue is paused and then unpaused, we call flush which by defaults recreate the isReadyPromise. + // After all the WRITE requests are done, the isReadyPromise is resolved, but since it's a new instance of promise, + // the pending READ request never received the resolved callback. That's why we don't want to recreate + // the promise when unpausing the queue. flush(false); } From 061dea7c6d8c561844c3dff6796101b8342ec7d0 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 25 Dec 2024 09:48:39 +0700 Subject: [PATCH 3/5] update comment --- src/libs/Network/SequentialQueue.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 0a0c5281a3a9..1af1337f845b 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -199,8 +199,8 @@ function unpause() { Log.info(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); isQueuePaused = false; - // A READ request will wait until all the WRITE requests are done, using isReadyPromise promise. - // When the queue is paused and then unpaused, we call flush which by defaults recreate the isReadyPromise. + // A READ request will wait until all the WRITE requests are done, using the isReadyPromise promise. + // When the queue is paused and then unpaused, we call flush which by defaults recreates the isReadyPromise. // After all the WRITE requests are done, the isReadyPromise is resolved, but since it's a new instance of promise, // the pending READ request never received the resolved callback. That's why we don't want to recreate // the promise when unpausing the queue. From 6db58742ac3ed4a3437df3f300fbe38bc42bae9e Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 27 Dec 2024 14:56:47 +0700 Subject: [PATCH 4/5] add comment --- src/libs/Network/SequentialQueue.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 1af1337f845b..2ddc6c5ae4cc 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -128,6 +128,10 @@ function process(): Promise { return currentRequestPromise; } +/** + * @param shouldResetPromise Determines whether the isReadyPromise should be reset. Resetting can cause unresolved READ requests + * to hang if tied to the old promise, so some cases (e.g., unpausing) require skipping the reset to maintain proper behavior. + */ function flush(shouldResetPromise = true) { // When the queue is paused, return early. This will keep an requests in the queue and they will get flushed again when the queue is unpaused if (isQueuePaused) { From 010c488699c33df3a5bc65d7d01ce149d497edb9 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 27 Dec 2024 14:59:02 +0700 Subject: [PATCH 5/5] update comment --- src/libs/Network/SequentialQueue.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 2ddc6c5ae4cc..c1c2b0946935 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -129,8 +129,10 @@ function process(): Promise { } /** - * @param shouldResetPromise Determines whether the isReadyPromise should be reset. Resetting can cause unresolved READ requests - * to hang if tied to the old promise, so some cases (e.g., unpausing) require skipping the reset to maintain proper behavior. + * @param shouldResetPromise Determines whether the isReadyPromise should be reset. + * A READ request will wait until all the WRITE requests are done, using the isReadyPromise promise. + * Resetting can cause unresolved READ requests to hang if tied to the old promise, + * so some cases (e.g., unpausing) require skipping the reset to maintain proper behavior. */ function flush(shouldResetPromise = true) { // When the queue is paused, return early. This will keep an requests in the queue and they will get flushed again when the queue is unpaused @@ -203,7 +205,6 @@ function unpause() { Log.info(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); isQueuePaused = false; - // A READ request will wait until all the WRITE requests are done, using the isReadyPromise promise. // When the queue is paused and then unpaused, we call flush which by defaults recreates the isReadyPromise. // After all the WRITE requests are done, the isReadyPromise is resolved, but since it's a new instance of promise, // the pending READ request never received the resolved callback. That's why we don't want to recreate