Skip to content

Commit 1ff47eb

Browse files
committed
Fix race condition during model selection
Previously the critical section did not include setting the MF_RUNNING flag for the selected model. With enough bad luck, a second thread might enter the critical section before the flag is set and pick up the exact same model. Additionally, the modification of current_model, which is shared across threads, was also not protected by the critical section.
1 parent c741dcb commit 1ff47eb

File tree

1 file changed

+24
-19
lines changed

1 file changed

+24
-19
lines changed

main/phylotesting.cpp

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2943,29 +2943,34 @@ CandidateModel CandidateModelSet::test(Params &params, PhyloTree* in_tree, Model
29432943
}
29442944

29452945
int64_t CandidateModelSet::getNextModel() {
2946-
int64_t next_model;
2947-
#pragma omp critical
2946+
uint64_t return_value;
2947+
2948+
#pragma omp critical
29482949
{
2949-
if (size() == 0)
2950-
next_model = -1;
2951-
else if (current_model == -1)
2952-
next_model = 0;
2953-
else {
2954-
for (next_model = current_model+1; next_model != current_model; next_model++) {
2955-
if (next_model == size())
2956-
next_model = 0;
2957-
if (!at(next_model).hasFlag(MF_IGNORED + MF_WAITING + MF_RUNNING)) {
2958-
break;
2950+
int64_t next_model;
2951+
2952+
if (empty()) {
2953+
next_model = -1;
2954+
} else {
2955+
for (next_model = current_model+1; next_model != current_model; next_model++) {
2956+
if (next_model == size())
2957+
next_model = 0;
2958+
if (!at(next_model).hasFlag(MF_IGNORED + MF_WAITING + MF_RUNNING)) {
2959+
break;
2960+
}
29592961
}
29602962
}
2963+
2964+
if (next_model != current_model) {
2965+
at(next_model).setFlag(MF_RUNNING);
2966+
current_model = next_model;
2967+
return_value = next_model;
2968+
} else {
2969+
return_value = -1;
2970+
}
29612971
}
2962-
}
2963-
if (next_model != current_model) {
2964-
current_model = next_model;
2965-
at(next_model).setFlag(MF_RUNNING);
2966-
return next_model;
2967-
} else
2968-
return -1;
2972+
2973+
return return_value;
29692974
}
29702975

29712976
CandidateModel CandidateModelSet::evaluateAll(Params &params, PhyloTree* in_tree, ModelCheckpoint &model_info,

0 commit comments

Comments
 (0)