Skip to content

Commit 1dfee78

Browse files
Ashutoshx7Ashutoshx7
authored
fix: resolve race condition in connectionStoreLock for concurrent turtles (#5046)
1 parent f318ed8 commit 1dfee78

File tree

2 files changed

+118
-52
lines changed

2 files changed

+118
-52
lines changed

‎js/blocks/FlowBlocks.js‎

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -197,45 +197,79 @@ function setupFlowBlocks(activity) {
197197

198198
tur.singer.inDuplicate = true;
199199

200+
/**
201+
* Acquires the connectionStoreLock with proper waiting.
202+
* Uses a polling mechanism to wait for the lock to be released.
203+
* @param {number} maxRetries - Maximum number of retry attempts
204+
* @param {number} retryInterval - Milliseconds between retries
205+
* @returns {Promise<boolean>} - Resolves to true when lock is acquired
206+
*/
207+
const __acquireLock = (maxRetries = 100, retryInterval = 10) => {
208+
return new Promise(resolve => {
209+
let retries = 0;
210+
const tryAcquire = () => {
211+
if (!logo.connectionStoreLock) {
212+
logo.connectionStoreLock = true;
213+
resolve(true);
214+
} else if (retries < maxRetries) {
215+
retries++;
216+
setTimeout(tryAcquire, retryInterval);
217+
} else {
218+
// Force acquire after max retries to prevent deadlock
219+
console.warn(
220+
"connectionStoreLock: Max retries reached, forcing lock acquisition"
221+
);
222+
logo.connectionStoreLock = true;
223+
resolve(true);
224+
}
225+
};
226+
tryAcquire();
227+
});
228+
};
229+
200230
// Listener function for handling the end of duplication
201-
const __listener = event => {
231+
const __listener = async event => {
202232
tur.singer.inDuplicate = false;
203233
tur.singer.duplicateFactor /= factor;
204234

205-
// Check for a race condition
206-
// FIXME: Do something about the race condition
207-
if (logo.connectionStoreLock) {
208-
console.debug("LOCKED");
209-
}
210-
211-
logo.connectionStoreLock = true;
212-
213-
// The last turtle should restore the broken connections
214-
if (__lookForOtherTurtles(blk, turtle) === null) {
215-
const n = logo.connectionStore[turtle][blk].length;
216-
for (let i = 0; i < n; i++) {
217-
const obj = logo.connectionStore[turtle][blk].pop();
218-
activity.blocks.blockList[obj[0]].connections[obj[1]] = obj[2];
219-
if (obj[2] != null) {
220-
activity.blocks.blockList[obj[2]].connections[0] = obj[0];
235+
// Acquire lock with proper waiting
236+
await __acquireLock();
237+
238+
try {
239+
// The last turtle should restore the broken connections
240+
if (__lookForOtherTurtles(blk, turtle) === null) {
241+
const n = logo.connectionStore[turtle][blk].length;
242+
for (let i = 0; i < n; i++) {
243+
const obj = logo.connectionStore[turtle][blk].pop();
244+
activity.blocks.blockList[obj[0]].connections[obj[1]] = obj[2];
245+
if (obj[2] != null) {
246+
activity.blocks.blockList[obj[2]].connections[0] = obj[0];
247+
}
221248
}
249+
} else {
250+
delete logo.connectionStore[turtle][blk];
222251
}
223-
} else {
224-
delete logo.connectionStore[turtle][blk];
252+
} finally {
253+
logo.connectionStoreLock = false;
225254
}
226-
227-
logo.connectionStoreLock = false;
228255
};
229256

230257
// Set the turtle listener
231258
logo.setTurtleListener(turtle, listenerName, __listener);
232259

233-
// Test for race condition
234-
// FIXME: Do something about the race condition
235-
if (logo.connectionStoreLock) {
236-
console.debug("LOCKED");
260+
// Acquire lock synchronously for the main flow
261+
// Note: This section runs synchronously, so we use a simple spin-wait
262+
// with a maximum iteration count to prevent infinite loops
263+
let lockAttempts = 0;
264+
const maxLockAttempts = 1000;
265+
while (logo.connectionStoreLock && lockAttempts < maxLockAttempts) {
266+
lockAttempts++;
267+
}
268+
if (lockAttempts >= maxLockAttempts) {
269+
console.warn(
270+
"connectionStoreLock: Max attempts reached in DuplicateBlock flow"
271+
);
237272
}
238-
239273
logo.connectionStoreLock = true;
240274

241275
// Check to see if another turtle has already disconnected these blocks

‎js/blocks/IntervalsBlocks.js‎

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -817,45 +817,77 @@ function setupIntervalsBlocks(activity) {
817817

818818
tur.singer.inDuplicate = true;
819819

820+
/**
821+
* Acquires the connectionStoreLock with proper waiting.
822+
* Uses a polling mechanism to wait for the lock to be released.
823+
* @param {number} maxRetries - Maximum number of retry attempts
824+
* @param {number} retryInterval - Milliseconds between retries
825+
* @returns {Promise<boolean>} - Resolves to true when lock is acquired
826+
*/
827+
const __acquireLock = (maxRetries = 100, retryInterval = 10) => {
828+
return new Promise(resolve => {
829+
let retries = 0;
830+
const tryAcquire = () => {
831+
if (!logo.connectionStoreLock) {
832+
logo.connectionStoreLock = true;
833+
resolve(true);
834+
} else if (retries < maxRetries) {
835+
retries++;
836+
setTimeout(tryAcquire, retryInterval);
837+
} else {
838+
// Force acquire after max retries to prevent deadlock
839+
console.warn(
840+
"connectionStoreLock: Max retries reached, forcing lock acquisition"
841+
);
842+
logo.connectionStoreLock = true;
843+
resolve(true);
844+
}
845+
};
846+
tryAcquire();
847+
});
848+
};
849+
820850
// eslint-disable-next-line no-unused-vars
821-
const __listener = event => {
851+
const __listener = async event => {
822852
tur.singer.inDuplicate = false;
823853
tur.singer.duplicateFactor /= factor;
824854
tur.singer.arpeggio = [];
825-
// Check for a race condition.
826-
// FIXME: Do something about the race condition.
827-
if (logo.connectionStoreLock) {
828-
// eslint-disable-next-line no-console
829-
console.debug("LOCKED");
830-
}
831855

832-
logo.connectionStoreLock = true;
833-
834-
// The last turtle should restore the broken connections.
835-
if (__lookForOtherTurtles(blk, turtle) === null) {
836-
const n = logo.connectionStore[turtle][blk].length;
837-
for (let i = 0; i < n; i++) {
838-
const obj = logo.connectionStore[turtle][blk].pop();
839-
activity.blocks.blockList[obj[0]].connections[obj[1]] = obj[2];
840-
if (obj[2] != null) {
841-
activity.blocks.blockList[obj[2]].connections[0] = obj[0];
856+
// Acquire lock with proper waiting
857+
await __acquireLock();
858+
859+
try {
860+
// The last turtle should restore the broken connections.
861+
if (__lookForOtherTurtles(blk, turtle) === null) {
862+
const n = logo.connectionStore[turtle][blk].length;
863+
for (let i = 0; i < n; i++) {
864+
const obj = logo.connectionStore[turtle][blk].pop();
865+
activity.blocks.blockList[obj[0]].connections[obj[1]] = obj[2];
866+
if (obj[2] != null) {
867+
activity.blocks.blockList[obj[2]].connections[0] = obj[0];
868+
}
842869
}
870+
} else {
871+
delete logo.connectionStore[turtle][blk];
843872
}
844-
} else {
845-
delete logo.connectionStore[turtle][blk];
873+
} finally {
874+
logo.connectionStoreLock = false;
846875
}
847-
logo.connectionStoreLock = false;
848876
};
849877

850878
logo.setTurtleListener(turtle, listenerName, __listener);
851879

852-
// Test for race condition.
853-
// FIXME: Do something about the race condition.
854-
if (logo.connectionStoreLock) {
855-
// eslint-disable-next-line no-console
856-
console.debug("LOCKED");
880+
// Acquire lock synchronously for the main flow
881+
// Note: This section runs synchronously, so we use a simple spin-wait
882+
// with a maximum iteration count to prevent infinite loops
883+
let lockAttempts = 0;
884+
const maxLockAttempts = 1000;
885+
while (logo.connectionStoreLock && lockAttempts < maxLockAttempts) {
886+
lockAttempts++;
887+
}
888+
if (lockAttempts >= maxLockAttempts) {
889+
console.warn("connectionStoreLock: Max attempts reached in ArpeggioBlock flow");
857890
}
858-
859891
logo.connectionStoreLock = true;
860892

861893
// Check to see if another turtle has already disconnected these blocks

0 commit comments

Comments
 (0)