Skip to content

Commit fd73208

Browse files
Tcp transport fixes
o Flip do-while loop for epoll return processing for both transmit and receive path. o Handle activateEndpoint failures. o Remove writeLock() in addEndpoint since all endpoint list management is alreay locked by global tcpTransport write lock. Signed-off-by: Bernard Metzler <[email protected]>
1 parent 20d24b4 commit fd73208

File tree

2 files changed

+25
-14
lines changed

2 files changed

+25
-14
lines changed

src/libgeds/TcpTransport.cpp

+20-13
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ void TcpTransport::stop() {
9090
if (eventFd > 0) {
9191
u_int64_t buf = 1;
9292
LOG_DEBUG("TCP Transport: write CTL fd");
93-
write(eventFd, &buf, 8);
93+
if (write(eventFd, &buf, 8) <= 0)
94+
perror("TCP Transport writing control socket failed: ");
9495
}
9596

9697
std::vector<std::shared_ptr<TcpPeer>> tcpPeerV;
@@ -334,7 +335,7 @@ void TcpTransport::tcpTxThread(unsigned int id) {
334335
perror("epoll_ctl: ");
335336
}
336337
}
337-
do {
338+
while (isServing) {
338339
int cnt = ::epoll_wait(poll_fd, events, EPOLL_MAXEVENTS, -1);
339340

340341
for (int i = 0; i < cnt; i++) {
@@ -383,7 +384,7 @@ void TcpTransport::tcpTxThread(unsigned int id) {
383384
tcpPeers.remove(tcpPeer->Id);
384385
}
385386
}
386-
} while (isServing);
387+
};
387388
if (eventFd > 0)
388389
epoll_ctl(poll_fd, EPOLL_CTL_DEL, eventFd, NULL);
389390

@@ -702,7 +703,7 @@ void TcpTransport::tcpRxThread(unsigned int id) {
702703
}
703704
}
704705

705-
do {
706+
while (isServing) {
706707
int cnt = ::epoll_wait(poll_fd, events, EPOLL_MAXEVENTS, -1);
707708

708709
for (int i = 0; i < cnt; i++) {
@@ -751,7 +752,7 @@ void TcpTransport::tcpRxThread(unsigned int id) {
751752
}
752753
}
753754
}
754-
} while (isServing);
755+
};
755756
if (eventFd > 0)
756757
epoll_ctl(poll_fd, EPOLL_CTL_DEL, eventFd, NULL);
757758

@@ -843,13 +844,16 @@ bool TcpTransport::addEndpointPassive(int sock) {
843844

844845
tep->sock = sock;
845846
tcpPeer->addEndpoint(tep);
846-
activateEndpoint(tep, tcpPeer);
847-
LOG_DEBUG("Server with ", tcpPeer->endpoints.size(),
848-
" connections to ", hostname, "::", in_peer->sin_port);
849-
return true;
847+
if (activateEndpoint(tep, tcpPeer) == true) {
848+
LOG_DEBUG("Server with ", tcpPeer->endpoints.size(),
849+
" connections to ", hostname, "::", in_peer->sin_port);
850+
return true;
851+
}
852+
tcpPeer->delEndpoint(tep);
853+
LOG_ERROR("Server failed adding connection to ", hostname, "::", in_peer->sin_port);
850854
}
851855
/*
852-
* Will kill this extra connection probably created during cross-connect
856+
* Drop this extra connection probably created during cross-connect
853857
* from both sides
854858
*/
855859
LOG_DEBUG("Server with ", tcpPeer->endpoints.size() + 1,
@@ -915,10 +919,13 @@ std::shared_ptr<TcpPeer> TcpTransport::getPeer(sockaddr *peer) {
915919
std::shared_ptr<TcpEndpoint> tep = std::make_shared<TcpEndpoint>();
916920
tep->sock = sock;
917921
tcpPeer->addEndpoint(tep);
918-
activateEndpoint(tep, tcpPeer);
919-
922+
if (activateEndpoint(tep, tcpPeer) == false) {
923+
tcpPeer->delEndpoint(tep);
924+
::close(sock);
925+
LOG_ERROR("Client failed adding connection to ", hostname, "::", inaddr->sin_port);
926+
break;
927+
}
920928
num_ep++;
921-
922929
LOG_DEBUG("Client with ", num_ep, " connections to ", hostname, "::", inaddr->sin_port);
923930
}
924931
if (num_ep)

src/libgeds/TcpTransport.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,13 @@ class TcpPeer : public std::enable_shared_from_this<TcpPeer>, utility::RWConcurr
182182
sendRpcRequest(uint64_t dest, std::string name, size_t src_off, size_t len);
183183
int sendRpcReply(uint64_t reqId, int in_fd, uint64_t start, size_t len, int status);
184184
void addEndpoint(std::shared_ptr<TcpEndpoint> tep) {
185-
auto lock = getWriteLock();
185+
// Caller must hold TcpTransports write lock
186186
endpoints.emplace(tep->sock, tep);
187187
};
188+
void delEndpoint(std::shared_ptr<TcpEndpoint> tep) {
189+
// Caller must hold TcpTransport write lock
190+
endpoints.erase(tep->sock);
191+
};
188192
TcpPeer(std::string name, std::shared_ptr<GEDS> geds, TcpTransport &tcpTransport)
189193
: Id(SStringHash(name)), _geds(std::move(geds)), _tcpTransport(tcpTransport),
190194
hostname(std::move(name)){};

0 commit comments

Comments
 (0)