Skip to content

Commit 6ed0f0c

Browse files
committed
fix race condition in ProtocolClient::Socket()
Signed-off-by: kostas <[email protected]>
1 parent 95ca692 commit 6ed0f0c

File tree

3 files changed

+10
-14
lines changed

3 files changed

+10
-14
lines changed

src/server/protocol_client.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,17 @@ ProtocolClient::ProtocolClient(string host, uint16_t port) {
109109
#ifdef DFLY_USE_SSL
110110
MaybeInitSslCtx();
111111
#endif
112+
113+
// We initialize the proactor thread here such that it never races with Sock().
114+
// ProtocolClient is never migrated to a different thread, so this is safe.
115+
socket_thread_ = ProactorBase::me();
112116
}
117+
113118
ProtocolClient::ProtocolClient(ServerContext context) : server_context_(std::move(context)) {
114119
#ifdef DFLY_USE_SSL
115120
MaybeInitSslCtx();
116121
#endif
122+
socket_thread_ = ProactorBase::me();
117123
}
118124

119125
ProtocolClient::~ProtocolClient() {
@@ -162,6 +168,7 @@ error_code ProtocolClient::ResolveHostDns() {
162168
error_code ProtocolClient::ConnectAndAuth(std::chrono::milliseconds connect_timeout_ms,
163169
ExecutionState* cntx) {
164170
ProactorBase* mythread = ProactorBase::me();
171+
DCHECK(mythread == socket_thread_);
165172
CHECK(mythread);
166173
{
167174
unique_lock lk(sock_mu_);

src/server/protocol_client.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class ProtocolClient {
107107
}
108108

109109
auto* Proactor() const {
110-
return sock_->proactor();
110+
return socket_thread_;
111111
}
112112

113113
util::FiberSocketBase* Sock() const {
@@ -142,6 +142,7 @@ class ProtocolClient {
142142
#else
143143
void* ssl_ctx_{nullptr};
144144
#endif
145+
util::fb2::ProactorBase* socket_thread_;
145146
};
146147

147148
} // namespace dfly

src/server/replica.cc

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,19 +1207,7 @@ auto Replica::GetSummary() const -> Summary {
12071207
return res;
12081208
};
12091209

1210-
if (Sock())
1211-
return Proactor()->AwaitBrief(f);
1212-
1213-
/**
1214-
* when this branch happens: there is a very short grace period
1215-
* where Sock() is not initialized, yet the server can
1216-
* receive ROLE/INFO commands. That period happens when launching
1217-
* an instance with '--replicaof' and then immediately
1218-
* sending a command.
1219-
*
1220-
* In that instance, we have to run f() on the current fiber.
1221-
*/
1222-
return f();
1210+
return Proactor()->AwaitBrief(f);
12231211
}
12241212

12251213
std::vector<uint64_t> Replica::GetReplicaOffset() const {

0 commit comments

Comments
 (0)