Skip to content

Volume destroy data race - #934

Open
nnastonen wants to merge 1 commit into
eBay:stable/v7.xfrom
nnastonen:SDSTOR-25815_volume_dstroy_data_race_bug_v7
Open

nnastonen wants to merge 1 commit into
eBay:stable/v7.xfrom
nnastonen:SDSTOR-25815_volume_dstroy_data_race_bug_v7

Conversation

@nnastonen

@nnastonen nnastonen commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

After volume destroy, the device is still in m_rd_map. A CP that fires in that window calls cp_flush on a device whose superblock is gone, and that null dereference causes the crash.

https://jirap.corp.ebay.com/browse/SDSTOR-25815

@nnastonen
nnastonen force-pushed the SDSTOR-25815_volume_dstroy_data_race_bug_v7 branch from b4f459d to cfa332d Compare October 1, 2026 09:00
@shosseinimotlagh
shosseinimotlagh requested a balanced review from Copilot October 1, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The concurrency fix lacks a deterministic regression test for checkpoint and device-removal overlap.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Prevents checkpoint callbacks from accessing a destroyed solo replication device.

Changes:

  • Removes devices from the replication map before resource destruction.
  • Waits for active device iterations to finish.
  • Bumps the package version and applies formatting updates.
File Description
generic_repl_svc.cpp Reorders device removal and destruction.
raft_repl_service.h Formatting only.
test_raft_repl_dev.cpp Formatting only.
conanfile.py Bumps version to 7.6.2.

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

Comment thread src/lib/replication/service/generic_repl_svc.cpp

@shosseinimotlagh shosseinimotlagh 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.

LG. @nnastonen please wait for @JacksonYao287 approval

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.

3 participants