Skip to content

Fix delayed task clock ordering - #8337

Merged
Amaury Chamayou (achamayou) merged 6 commits into
mainfrom
achamayou-fix-issue-8294
Sep 11, 2026
Merged

Fix delayed task clock ordering#8337
Amaury Chamayou (achamayou) merged 6 commits into
mainfrom
achamayou-fix-issue-8294

Conversation

@achamayou

@achamayou Amaury Chamayou (achamayou) commented Sep 9, 2026

Copy link
Copy Markdown
Member

Summary

  • serialize the delayed-task logical clock with the delayed-task deadline map
  • publish each tick's new logical time before making expired tasks runnable
  • ensure follow-up tasks schedule their deadlines relative to the tick that released their parent

Closes #8294

Bug and fix

Before: the parent can sample a stale clock

%%{init: {"flowchart": {"curve": "basis", "nodeSpacing": 32, "rankSpacing": 42}, "themeVariables": {"fontFamily": "ui-sans-serif, system-ui, sans-serif"}}}%%
flowchart TB
  subgraph Ticker["Ticker lane"]
    direction TB
    B1("Sample A: read clock = 0<br/>job_board.cpp:171 pre-fix")
    B2("Lock deadline map<br/>job_board.cpp:174 pre-fix")
    B3("Make parent runnable<br/>job_board.cpp:192 pre-fix")
    B4("Unlock deadline map<br/>job_board.cpp:212 pre-fix")
    B8("Publish clock = 100<br/>job_board.cpp:214 pre-fix")
    B1 --> B2 --> B3 --> B4 --> B8
  end

  subgraph Parent["Parent-task lane"]
    direction TB
    B5("Parent starts and waits for deadline-map mutex")
    B6("Lock deadline map<br/>job_board.cpp:163 pre-fix")
    B7("Sample B: read clock = 0<br/>job_board.cpp:165 pre-fix")
    B75("Schedule child at 0 + 10 = 10 ms")
    B9("At logical time 101 ms<br/>child is released 9 ms too early")
    B5 --> B6 --> B7 --> B75 --> B9
  end

  B3 -. "parent becomes runnable" .-> B5
  B4 -. "mutex handoff" .-> B6
  B75 -. "ticker resumes only afterward" .-> B8

  classDef ticker fill:#ddf4ff,stroke:#54aeff,color:#0550ae,stroke-width:1.5px
  classDef parent fill:#fbefff,stroke:#bf8fff,color:#6639ba,stroke-width:1.5px
  classDef sample fill:#fff8c5,stroke:#d4a72c,color:#633c01,stroke-width:2px
  classDef bug fill:#ffebe9,stroke:#ff8182,color:#82071e,stroke-width:2px
  class B2,B3,B4,B8 ticker
  class B5,B6 parent
  class B1,B7 sample
  class B75,B9 bug
  style Ticker fill:#f6f8fa,stroke:#54aeff,stroke-width:2px,color:#1f2328
  style Parent fill:#f6f8fa,stroke:#bf8fff,stroke-width:2px,color:#1f2328
  linkStyle default stroke:#8c959f,stroke-width:1.5px

  click B1 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L171" "Open pre-fix job_board.cpp:171"
  click B2 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L174" "Open pre-fix job_board.cpp:174"
  click B3 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L192" "Open pre-fix job_board.cpp:192"
  click B4 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L212" "Open pre-fix job_board.cpp:212"
  click B6 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L163" "Open pre-fix job_board.cpp:163"
  click B7 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L165" "Open pre-fix job_board.cpp:165"
  click B8 "https://github.com/microsoft/CCF/blob/d2a49a4250805112aab588696443af1332efae5a/src/tasks/job_board.cpp#L214" "Open pre-fix job_board.cpp:214"
Loading

After: the shared mutex orders both samples

%%{init: {"flowchart": {"curve": "basis", "nodeSpacing": 32, "rankSpacing": 42}, "themeVariables": {"fontFamily": "ui-sans-serif, system-ui, sans-serif"}}}%%
flowchart TB
  subgraph Ticker["Ticker lane"]
    direction TB
    F1("Lock clock and deadline map<br/>job_board.cpp:171 fixed")
    F2("Sample A: read clock = 0<br/>job_board.cpp:172 fixed")
    F3("Publish clock = 100 while still locked<br/>job_board.cpp:173 fixed")
    F4("Make parent runnable<br/>job_board.cpp:192 fixed")
    F5("Unlock shared state<br/>job_board.cpp:212 fixed")
    F1 --> F2 --> F3 --> F4 --> F5
  end

  subgraph Parent["Parent-task lane"]
    direction TB
    F6("Parent starts and waits for shared mutex")
    F7("Lock shared state<br/>job_board.cpp:162 fixed")
    F8("Sample B: read clock = 100<br/>job_board.cpp:164 fixed")
    F9("Schedule child at 100 + 10 = 110 ms")
    F10("Child remains pending at 101 ms<br/>and is released at 110 ms")
    F6 --> F7 --> F8 --> F9 --> F10
  end

  F4 -. "parent becomes runnable" .-> F6
  F5 -. "mutex handoff" .-> F7

  classDef ticker fill:#ddf4ff,stroke:#54aeff,color:#0550ae,stroke-width:1.5px
  classDef parent fill:#fbefff,stroke:#bf8fff,color:#6639ba,stroke-width:1.5px
  classDef sample fill:#dafbe1,stroke:#4ac26b,color:#116329,stroke-width:2px
  classDef outcome fill:#dafbe1,stroke:#4ac26b,color:#116329,stroke-width:2px
  class F1,F3,F4,F5 ticker
  class F6,F7 parent
  class F2,F8 sample
  class F9,F10 outcome
  style Ticker fill:#f6f8fa,stroke:#54aeff,stroke-width:2px,color:#1f2328
  style Parent fill:#f6f8fa,stroke:#bf8fff,stroke-width:2px,color:#1f2328
  linkStyle default stroke:#8c959f,stroke-width:1.5px

  click F1 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L171" "Open fixed job_board.cpp:171"
  click F2 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L172" "Open fixed job_board.cpp:172"
  click F3 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L173" "Open fixed job_board.cpp:173"
  click F4 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L192" "Open fixed job_board.cpp:192"
  click F5 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L212" "Open fixed job_board.cpp:212"
  click F7 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L162" "Open fixed job_board.cpp:162"
  click F8 "https://github.com/microsoft/CCF/blob/52ea5f1d6bf8127b0cae05adf55520be33b2315e/src/tasks/job_board.cpp#L164" "Open fixed job_board.cpp:164"
Loading

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 20:28
@achamayou
Amaury Chamayou (achamayou) requested a review from a team as a code owner September 9, 2026 20:28
@achamayou Amaury Chamayou (achamayou) added the run-long-test Run Long Test job label Sep 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A regression test should be added to cover the specific concurrent interleaving described in #8294 so the fixed ordering cannot silently regress.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes a concurrency ordering bug in the task system’s delayed-task scheduling by making the delayed-task logical clock (total_elapsed) consistent with the delayed-task deadline map and publishing the updated clock before releasing expired tasks to workers.

Changes:

  • Move delayed.total_elapsed from an atomic to a mutex-guarded field, serialized with delayed.tasks.
  • Update tick() to advance/publish the delayed-task logical time while holding delayed.tasks_mutex, before enqueueing expired tasks.
  • Ensure new delayed deadlines are computed from the serialized logical clock under the same mutex.

Custom instructions used:

  • /.github/copilot-instructions.md
  • /.github/instructions/reviewing.instructions.md
File summaries
File Description
src/tasks/job_board.cpp Serializes the delayed-task logical clock with the delayed-task map and updates ordering in tick() to prevent follow-up tasks being scheduled against a stale clock.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tasks/job_board.cpp
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@achamayou
Amaury Chamayou (achamayou) merged commit 5bcca65 into main Sep 11, 2026
13 checks passed
@achamayou
Amaury Chamayou (achamayou) deleted the achamayou-fix-issue-8294 branch September 11, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delayed follow-up tasks can fire before their requested delay

4 participants