Skip to content

fix: await queue lock operations and fix variable name bug#71

Open
Its-donkey wants to merge 1 commit intoopti21:mainfrom
Its-donkey:fix/queue-race-condition-and-request-check
Open

fix: await queue lock operations and fix variable name bug#71
Its-donkey wants to merge 1 commit intoopti21:mainfrom
Its-donkey:fix/queue-race-condition-and-request-check

Conversation

@Its-donkey
Copy link

Summary

This PR fixes two critical bugs discovered during a security audit:

  1. Race condition in queue operations - Lock/unlock functions were not being awaited
  2. Variable name typo - Error handling was checking the wrong variable and never triggering

Changes

1. Queue Lock Race Condition (src/redis/handlers/Queue.ts)

Before:

lockQueue();        // Not awaited - returns immediately
await connect();
// ... database operations ...
unLockQueue();      // Not awaited - returns immediately

After:

await lockQueue();  // Properly waits for lock
await connect();
// ... database operations ...
await unLockQueue(); // Properly waits for unlock

Functions fixed:

  • addToQueue()
  • removeFromOrder()
  • updateOrder()
  • updateOrderIdStrings()
  • updateNowPlaying()

Impact:

Scenario Before After
Two users submit songs simultaneously Both requests could modify queue at same time, causing data corruption, duplicate entries, or lost requests Requests are properly serialized through the lock mechanism
High traffic during stream Queue order could become inconsistent, songs could disappear from queue Queue operations are atomic and consistent
Now playing updates Race between update and next song could cause wrong song to display Updates complete before next operation begins

2. Variable Name Bug (src/commands/songRequest.ts)

Before (line 142):

const createdRequest = await createRequest(...);

if (!createRequest) {  // ❌ Checks the FUNCTION (always truthy)
  twitch.say(channel, `Error creating request DinkDank @opti_21`);
  return;
}

After:

const createdRequest = await createRequest(...);

if (!createdRequest) {  // ✅ Checks the RESULT variable
  twitch.say(channel, `Error creating request DinkDank @opti_21`);
  return;
}

Impact:

Scenario Before After
createRequest() returns null/undefined Error silently ignored, code continues with undefined data Error properly caught, user notified, function exits
Database error during request creation Undefined request passed to addToQueue(), potential crash or data corruption Error handled gracefully with user feedback
Debugging failed requests No error message shown, difficult to diagnose Clear error message in chat mentioning @opti_21

Test Plan

  • Submit a song request when video exists in DB - verify request is created and added to queue
  • Simulate a failed request creation - verify error message appears in chat
  • Submit multiple song requests rapidly - verify queue order is consistent
  • Test queue operations (add, remove, reorder) under load - verify no race conditions

Risk Assessment

Low risk - These are bug fixes that make the existing code work as originally intended. No new functionality or architectural changes.

❤️ BarneyoftheRubble

- Add await to lockQueue()/unLockQueue() calls to prevent race conditions
  in addToQueue, removeFromOrder, updateOrder, updateOrderIdStrings,
  and updateNowPlaying functions
- Fix typo checking wrong variable (!createRequest -> !createdRequest)
  in songRequest handler that caused error handling to never trigger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant