Skip to content

Commit bbcc8c7

Browse files
snejpasin
authored andcommitted
CBL-8580: Use AtomicRetained in Replicator::Subreplicators
Fixes a race condition when a Replicator is torn down. --------- Co-authored-by: Pasin Suriyentrakorn <pasin@couchbase.com>
1 parent 1587e91 commit bbcc8c7

2 files changed

Lines changed: 23 additions & 16 deletions

File tree

Replicator/Replicator.cc

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,13 @@ namespace litecore::repl {
252252

253253
// Called after the checkpoint is established.
254254
void Replicator::startReplicating(CollectionIndex coll) {
255-
if ( _options->push(coll) > kC4Passive ) _subRepls[coll].pusher->start();
256-
if ( _options->pull(coll) > kC4Passive )
257-
_subRepls[coll].puller->start(_subRepls[coll].checkpointer->remoteMinSequence());
255+
if ( _options->push(coll) > kC4Passive ) {
256+
if ( auto pusher = _subRepls[coll].pusher.get() ) pusher->start();
257+
}
258+
if ( _options->pull(coll) > kC4Passive ) {
259+
if ( auto puller = _subRepls[coll].puller.get() )
260+
puller->start(_subRepls[coll].checkpointer->remoteMinSequence());
261+
}
258262
}
259263

260264
pair<int, websocket::Headers> Replicator::httpResponse() const {
@@ -379,9 +383,9 @@ namespace litecore::repl {
379383

380384
CollectionIndex coll = task->collectionIndex();
381385
if ( coll != kNotCollectionIndex ) {
382-
if ( task == _subRepls[coll].pusher ) {
386+
if ( auto pusher = _subRepls[coll].pusher.get(); pusher == task ) {
383387
updatePushStatus(coll, taskStatus);
384-
} else if ( task == _subRepls[coll].puller ) {
388+
} else if ( auto puller = _subRepls[coll].puller.get(); puller == task ) {
385389
updatePullStatus(coll, taskStatus);
386390
}
387391
}
@@ -656,8 +660,8 @@ namespace litecore::repl {
656660
// Clear connection() and notify the other agents to do the same:
657661
_connectionClosed();
658662
for ( auto& sub : _subRepls ) {
659-
if ( sub.pusher ) sub.pusher->connectionClosed();
660-
if ( sub.puller ) sub.puller->connectionClosed();
663+
if ( auto pusher = sub.pusher.get() ) pusher->connectionClosed();
664+
if ( auto puller = sub.puller.get() ) puller->connectionClosed();
661665
}
662666

663667
if ( status.isNormal() && closedByPeer && _options->isActive() ) {
@@ -718,9 +722,10 @@ namespace litecore::repl {
718722
cLogInfo(coll, "No local checkpoint '%.*s'", SPLAT(sub.checkpointer->initialCheckpointID()));
719723
// If pulling into an empty db with no checkpoint, it's safe to skip deleted
720724
// revisions as an optimization.
721-
if ( _options->pull(coll) > kC4Passive && sub.puller
725+
if ( auto puller = sub.puller.get();
726+
puller && _options->pull(coll) > kC4Passive
722727
&& _db->useCollection(sub.collectionSpec)->getLastSequence() == 0_seq )
723-
sub.puller->setSkipDeleted();
728+
puller->setSkipDeleted();
724729
}
725730
return true;
726731
} catch ( ... ) {
@@ -767,8 +772,9 @@ namespace litecore::repl {
767772

768773
if ( !refresh && sub.hadLocalCheckpoint ) {
769774
// Compare checkpoints, reset if mismatched:
770-
bool valid = sub.checkpointer->validateWith(remoteCheckpoint);
771-
if ( !valid && sub.pusher ) sub.pusher->checkpointIsInvalid();
775+
if ( !sub.checkpointer->validateWith(remoteCheckpoint) ) {
776+
if ( auto pusher = sub.pusher.get() ) pusher->checkpointIsInvalid();
777+
}
772778

773779
if ( !refresh ) {
774780
// Now we have the checkpoints! Time to start replicating:
@@ -898,8 +904,9 @@ namespace litecore::repl {
898904

899905
if ( _subRepls[i].hadLocalCheckpoint ) {
900906
// Compare checkpoints, reset if mismatched:
901-
bool valid = _subRepls[i].checkpointer->validateWith(remoteCheckpoints[i]);
902-
if ( !valid && _subRepls[i].pusher ) _subRepls[i].pusher->checkpointIsInvalid();
907+
if ( !_subRepls[i].checkpointer->validateWith(remoteCheckpoints[i]) ) {
908+
if ( auto pusher = _subRepls[i].pusher.get() ) pusher->checkpointIsInvalid();
909+
}
903910
}
904911
// Now we have the checkpoints! Time to start replicating:
905912
startReplicating(i);

Replicator/Replicator.hh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@
1111
//
1212

1313
#pragma once
14+
#include "AtomicRetained.hh"
1415
#include "Worker.hh"
1516
#include "Checkpointer.hh"
1617
#include "BLIPConnection.hh"
1718
#include "Batcher.hh"
1819
#include "Stopwatch.hh"
1920
#include "c4DatabaseTypes.h"
2021
#include <access_lock.hh>
21-
#include <array>
2222
#include <optional>
2323
#include <utility>
2424

@@ -232,8 +232,8 @@ namespace litecore::repl {
232232
// Member variables:
233233

234234
struct SubReplicator {
235-
Retained<Pusher> pusher;
236-
Retained<Puller> puller;
235+
AtomicRetained<Pusher> pusher;
236+
AtomicRetained<Puller> puller;
237237
Status pushStatus; // Current status of Pusher
238238
Status pullStatus; // Current status of Puller
239239
unique_ptr<Checkpointer> checkpointer; // Object that manages checkpoints

0 commit comments

Comments
 (0)