Skip to content

fix(duplication): prevent potential crash when removing or pausing duplication #2368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lengyuexuexuan wants to merge 5 commits into
base: master
Choose a base branch
from
+16 −7
Open

fix(duplication): prevent potential crash when removing or pausing duplication #2368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f0acd67
fix(duplication): fix race condition in _duplications access
Feb 12, 2026
e2349bb
fix(duplication): fix potential core of duplication during primary re…
Feb 12, 2026
871b37d
style: format task.h with clang-format
Feb 12, 2026
15f1dfa
style: fix task.h
Feb 26, 2026
da7b188
style: fix replica_duplicator_manager.cpp
Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/replica/duplication/replica_duplicator_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,14 @@ replica_duplicator_manager::get_duplication_confirms_to_update() const

void replica_duplicator_manager::sync_duplication(const duplication_entry &ent)
{
zauto_lock l(_lock);
auto it = ent.progress.find(get_gpid().get_partition_index());
if (it == ent.progress.end()) {
// Inconsistent with the meta server.
_duplications.erase(ent.dupid);
return;
}

zauto_lock l(_lock);

dupid_t dupid = ent.dupid;
duplication_status::type next_status = ent.status;

Expand Down Expand Up @@ -183,7 +182,6 @@ void replica_duplicator_manager::update_confirmed_decree_if_secondary(decree con
return;
}

zauto_lock l(_lock);
Copy link
Contributor

@empiredan empiredan Mar 6, 2026

Choose a reason for hiding this comment

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

Does _primary_confirmed_decree below no longer need protection by _lock?

remove_all_duplications();
if (confirmed >= 0) { // duplication ongoing
// confirmed decree never decreases
Expand All @@ -197,7 +195,8 @@ void replica_duplicator_manager::update_confirmed_decree_if_secondary(decree con

void replica_duplicator_manager::METRIC_FUNC_NAME_SET(dup_pending_mutations)()
{
int64_t total = 0;
zauto_lock l(_lock);
uint64_t total = 0;
for (const auto &dup : _duplications) {
total += dup.second->get_pending_mutations_count();
}
Expand Down Expand Up @@ -227,6 +226,7 @@ replica_duplicator_manager::get_dup_states() const

void replica_duplicator_manager::remove_all_duplications()
{
zauto_lock l(_lock);
// fast path
if (_duplications.empty()) {
return;
Expand Down
15 changes: 12 additions & 3 deletions src/task/task.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "utils/fmt_logging.h"
#include "utils/join_point.h"
#include "utils/ports.h"
#include "utils/synchronize.h"
#include "utils/utils.h"

namespace dsn {
Expand Down Expand Up @@ -329,6 +330,11 @@ class raw_task : public task
: task(code, hash, node), _cb(std::move(cb))
{
}
~raw_task() override
{
::dsn::utils::auto_lock<::dsn::utils::ex_lock_nr> l(_lock);
_cb = nullptr;
}

void exec() override
{
Expand All @@ -338,10 +344,13 @@ class raw_task : public task
}

protected:
void clear_non_trivial_on_task_end() override { _cb = nullptr; }

protected:
void clear_non_trivial_on_task_end() override
{
::dsn::utils::auto_lock<::dsn::utils::ex_lock_nr> l(_lock);
_cb = nullptr;
}
task_handler _cb;
::dsn::utils::ex_lock_nr _lock;
};

//----------------- timer task -------------------------------------------------------
Expand Down
Loading