Skip to content

Commit 1292677

Browse files
zmurjdduncan
authored andcommitted
Bug#35703454 NdbSocket refactoring - move invalidate into close [NOCLOSE]
NdbSocket::close should always invalidate the socket file descriptor. Renamed invalidate into a private method invalidate_socket_handle and used that in close and other methods. Change-Id: Ic02406a72de3e452287337dcc87f8a0d361ecf8a
1 parent 4468712 commit 1292677

File tree

11 files changed

+20
-27
lines changed

11 files changed

+20
-27
lines changed

storage/ndb/include/util/NdbSocket.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ class NdbSocket {
4848

4949
void init_from_new(ndb_socket_t);
5050
void init_from_native(socket_t fd) { ndb_socket_init_from_native(s, fd); }
51-
void invalidate();
5251
int is_valid() const { return ndb_socket_valid(s); }
5352
bool has_tls() const { return ssl; }
5453
ndb_socket_t ndb_socket() const { return s; }
@@ -169,6 +168,8 @@ class NdbSocket {
169168
bool check_hup() const;
170169

171170
private:
171+
void invalidate_socket_handle();
172+
172173
ssize_t ssl_recv(char * buf, size_t len) const;
173174
ssize_t ssl_peek(char * buf, size_t len) const;
174175
ssize_t ssl_send(const char * buf, size_t len) const;
@@ -229,9 +230,10 @@ NdbSocket& NdbSocket::operator=(NdbSocket && oth) {
229230
}
230231

231232
inline
232-
void NdbSocket::invalidate() {
233-
assert(ssl == nullptr); // call close() before invalidate()
234-
assert(mutex == nullptr); // call close() before invalidate()
233+
void NdbSocket::invalidate_socket_handle() {
234+
// call close() before invalidate_socket_handle()
235+
assert(ssl == nullptr);
236+
assert(mutex == nullptr);
235237
ndb_socket_invalidate(&s);
236238
}
237239

@@ -268,13 +270,17 @@ inline
268270
int NdbSocket::close() {
269271
if(ssl) ssl_close();
270272
disable_locking();
271-
return ndb_socket_close(s);
273+
int r = ndb_socket_close(s);
274+
invalidate_socket_handle();
275+
return r;
272276
}
273277

274278
inline
275279
void NdbSocket::close_with_reset(bool with_reset) {
276280
if(ssl) ssl_close();
281+
disable_locking();
277282
ndb_socket_close_with_reset(s, with_reset);
283+
invalidate_socket_handle();
278284
}
279285

280286
/* ndb_socket_poller.h */

storage/ndb/src/common/transporter/Loopback_Transporter.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,19 @@ Loopback_Transporter::connect_client()
7474
void
7575
Loopback_Transporter::disconnectImpl()
7676
{
77-
ndb_socket_t pair[] = { theSocket.ndb_socket(), m_send_socket };
78-
7977
get_callback_obj()->lock_transporter(m_transporter_index);
8078

81-
theSocket.invalidate();
79+
NdbSocket recv_sock = std::move(theSocket);
80+
ndb_socket_t send_sock = m_send_socket;
8281
ndb_socket_invalidate(&m_send_socket);
8382

8483
get_callback_obj()->unlock_transporter(m_transporter_index);
8584

86-
if (ndb_socket_valid(pair[0]))
87-
ndb_socket_close(pair[0]);
85+
if (recv_sock.is_valid())
86+
recv_sock.close();
8887

89-
if (ndb_socket_valid(pair[1]))
90-
ndb_socket_close(pair[1]);
88+
if (ndb_socket_valid(send_sock))
89+
ndb_socket_close(send_sock);
9190
}
9291

9392
bool

storage/ndb/src/common/transporter/SHM_Transporter.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,6 @@ SHM_Transporter::disconnect_socket()
623623
report_error(TE_ERROR_CLOSING_SOCKET);
624624
}
625625
}
626-
theSocket.invalidate();
627626
setupBuffersUndone();
628627
get_callback_obj()->unlock_transporter(m_transporter_index);
629628
}

storage/ndb/src/common/transporter/TCP_Transporter.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,6 @@ TCP_Transporter::shutdown()
485485
DEB_MULTI_TRP(("Close socket for trp %u",
486486
getTransporterIndex()));
487487
theSocket.close();
488-
theSocket.invalidate();
489488
}
490489
else
491490
{

storage/ndb/src/common/transporter/Transporter.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ bool Transporter::do_disconnect(int err, bool send_source)
183183
DEB_MULTI_TRP(("Close trp_id %u in inactive mode, socket valid",
184184
getTransporterIndex()));
185185
theSocket.close();
186-
theSocket.invalidate();
187186
}
188187
else
189188
{

storage/ndb/src/common/util/NdbSocket.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ bool NdbSocket::ssl_handshake() {
177177

178178
log_ssl_error(desc);
179179
close();
180-
invalidate();
181180
return false;
182181
}
183182

storage/ndb/src/common/util/SocketClient.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ SocketClient::authenticate(NdbSocket & secureSocket)
226226
g_eventLogger->error("Socket authentication failed: %s\n",
227227
m_auth->error(r));
228228
secureSocket.close();
229-
secureSocket.invalidate();
230229
}
231230
return r;
232231
}

storage/ndb/src/common/util/testSecureSocket.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,11 @@ Client::Client(const char * hostname) : SocketClient(nullptr),
319319
}
320320

321321
NdbSocket & Client::connect_plain() {
322+
require(!m_plain_socket.is_valid());
322323
ndb_sockaddr server_addr;
323324
if (Ndb_getAddr(&server_addr, m_server_host))
324325
{
325326
puts("Plain connection lookup server address failed.");
326-
m_plain_socket.invalidate();
327327
return m_plain_socket;
328328
}
329329
server_addr.set_port(opt_port);
@@ -335,11 +335,11 @@ NdbSocket & Client::connect_plain() {
335335
}
336336

337337
NdbSocket & Client::connect_tls() {
338+
require(!m_tls_socket.is_valid());
338339
ndb_sockaddr server_addr;
339340
if (Ndb_getAddr(&server_addr, m_server_host))
340341
{
341342
puts("TLS connection lookup server address failed.");
342-
m_tls_socket.invalidate();
343343
return m_tls_socket;
344344
}
345345
server_addr.set_port(opt_port + 1);
@@ -357,7 +357,7 @@ NdbSocket & Client::connect_tls() {
357357
}
358358
else NdbSocket::free_ssl(ssl);
359359
puts("TLS connection failed.");
360-
m_tls_socket.invalidate();
360+
m_tls_socket.close();
361361
}
362362
ndb_setsockopt(m_tls_socket.ndb_socket(), IPPROTO_TCP, TCP_NODELAY,
363363
& opt_tcp_no_delay);

storage/ndb/src/cw/cpcd/APIService.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ void CPCDAPISession::runSession() {
172172
}
173173
}
174174
m_secure_socket.close();
175-
m_secure_socket.invalidate();
176175
}
177176

178177
void CPCDAPISession::stopSession() {

storage/ndb/src/mgmapi/mgmapi.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,6 @@ int ndb_mgm_is_connected(NdbMgmHandle handle)
678678
{
679679
handle->connected= 0;
680680
handle->socket.close();
681-
handle->socket.invalidate();
682681
}
683682
}
684683
return handle->connected;
@@ -1019,7 +1018,6 @@ int
10191018
ndb_mgm_disconnect_quiet(NdbMgmHandle handle)
10201019
{
10211020
handle->socket.close();
1022-
handle->socket.invalidate();
10231021
handle->connected = 0;
10241022

10251023
return 0;

storage/ndb/src/mgmsrv/Services.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,6 @@ MgmApiSession::~MgmApiSession()
397397
if(m_secure_socket.is_valid())
398398
{
399399
m_secure_socket.close();
400-
m_secure_socket.invalidate();
401400
}
402401
if(m_stopSelf < 0)
403402
g_RestartServer= true;
@@ -492,7 +491,6 @@ MgmApiSession::runSession()
492491
if(m_secure_socket.is_valid())
493492
{
494493
m_secure_socket.close();
495-
m_secure_socket.invalidate();
496494
}
497495
NdbMutex_Unlock(m_mutex);
498496

@@ -1860,7 +1858,6 @@ MgmApiSession::listen_event(Parser<MgmApiSession>::Context & ctx,
18601858
{
18611859
m_mgmsrv.m_event_listner.add_listener(le);
18621860
m_stop = true;
1863-
m_secure_socket.invalidate();
18641861
}
18651862
}
18661863

@@ -1908,7 +1905,6 @@ MgmApiSession::transporter_connect(Parser_t::Context &ctx,
19081905
}
19091906
// Close the socket to indicate failure to client
19101907
m_secure_socket.close_with_reset(close_with_reset);
1911-
m_secure_socket.invalidate(); // Already closed
19121908
}
19131909
else
19141910
{

0 commit comments

Comments
 (0)