Skip to content

refactor(bthread): make TaskControl metric ownership explicit - #3490

Open
darion-yaphet wants to merge 1 commit into
apache:masterfrom
darion-yaphet:fix/task-control-bvar-ownership
Open

refactor(bthread): make TaskControl metric ownership explicit#3490
darion-yaphet wants to merge 1 commit into
apache:masterfrom
darion-yaphet:fix/task-control-bvar-ownership

Conversation

@darion-yaphet

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Problem Summary:

TaskControl stored per-tag bvar metrics as raw pointers. In addition, each cumulative-time PassiveStatus received a heap-allocated callback argument with no explicit owner. Since PassiveStatus does not own that argument, its lifetime was unclear and could leak when TaskControl is destroyed.

What is changed and the side effects?

Changed:

  • Replace the four per-tag bvar raw-pointer vectors with std::unique_ptr containers.
  • Add explicit ownership for each CumulatedWithTagArgs callback argument.
  • Preserve metric names, access patterns, and scheduler behavior.
  • Keep callback arguments alive until their corresponding PassiveStatus is destroyed.

Side effects:

  • Performance effects: No steady-state impact expected. Ownership changes occur only during TaskControl initialization/destruction.
  • Breaking backward compatibility: None; no public API or metric name changes.

———

Check List:

  • The modified task_control.cpp CMake object target compiles successfully.
  • git diff --check passes.
  • Full bthread test suite was not run because the current build is blocked by an unrelated brpc::EPROGREADTIMEOUT compilation error.
  • No new feature; no additional behavior test required.

Per-tag bvar metrics and cumulative-time callback arguments were created with raw pointers, leaving destruction responsibilities unclear. Store them with unique_ptr while preserving metric names, access patterns, and scheduler behavior.

Constraint: PassiveStatus does not own its callback argument

Rejected: vector<CumulatedWithTagArgs> | reallocation can invalidate callback addresses

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep callback arguments alive until after their PassiveStatus is destroyed

Tested: Rebuilt task_control.cpp CMake object target; git diff --check

Not-tested: Full bthread tests are blocked by the existing EPROGREADTIMEOUT build error
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