Skip to content

Conversation

@Boat2U
Copy link
Contributor

@Boat2U Boat2U commented Dec 11, 2025

Title

Type

  • feat: (new feature)
  • fix: (bug fix)
  • docs: (doc update)
  • refactor: (refactor code)
  • test: (test code)
  • chore: (other updates)

Scope

  • query: (query engine)
    • parser: (frontend parser)
    • planner: (frontend planner)
    • optimizer: (query optimizer)
    • executor: (execution engine)
    • op: (operators)
  • storage: (storage engine)
    • mvcc: (multi version concurrency control)
    • schema: (graph model and topology)
  • tool: (tools)
    • cli: (cli)
    • sdk: (sdk)
  • none: (N/A)

Description

Issue: #131

Checklist

  • I have prepared the pull request title according to the requirements.
  • I have successfully run all unit tests and integration tests.
  • I have already rebased the latest master branch.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.

Copilot AI review requested due to automatic review settings December 11, 2025 15:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical race condition and spurious wakeup issues in the ConcurrentQueue implementation. The fix removes a separate condition variable mutex that created a race condition where push notifications could be lost, and replaces the wait mechanism with a predicate-based approach that guards against spurious wakeups.

Key Changes

  • Removed the redundant c: Mutex<bool> field that was causing a race condition
  • Changed wait_for_push_notify to use wait_timeout_while with a predicate instead of wait_timeout, ensuring the wait condition is checked atomically with the same mutex that protects the queue state
  • Added a clarifying comment explaining the spurious wakeup protection

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@liuys-dase
Copy link
Contributor

LGTM

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.

2 participants