Skip to content

Commit e35f76e

Browse files
committed
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 0f33319 commit e35f76e

File tree

11 files changed

+20
-26
lines changed

11 files changed

+20
-26
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; }
@@ -170,6 +169,8 @@ class NdbSocket {
170169
bool check_hup() const;
171170

172171
private:
172+
void invalidate_socket_handle();
173+
173174
ssize_t ssl_recv(char * buf, size_t len) const;
174175
ssize_t ssl_peek(char * buf, size_t len) const;
175176
ssize_t ssl_send(const char * buf, size_t len) const;
@@ -233,9 +234,10 @@ NdbSocket& NdbSocket::operator=(NdbSocket && oth) {
233234
}
234235

235236
inline
236-
void NdbSocket::invalidate() {
237-
assert(ssl == nullptr); // call close() before invalidate()
238-
assert(mutex == nullptr); // call close() before invalidate()
237+
void NdbSocket::invalidate_socket_handle() {
238+
// call close() before invalidate_socket_handle()
239+
assert(ssl == nullptr);
240+
assert(mutex == nullptr);
239241
ndb_socket_invalidate(&s);
240242
}
241243

@@ -272,13 +274,17 @@ inline
272274
int NdbSocket::close() {
273275
if(ssl) ssl_close();
274276
disable_locking();
275-
return ndb_socket_close(s);
277+
int r = ndb_socket_close(s);
278+
invalidate_socket_handle();
279+
return r;
276280
}
277281

278282
inline
279283
void NdbSocket::close_with_reset(bool with_reset) {
280284
if(ssl) ssl_close();
285+
disable_locking();
281286
ndb_socket_close_with_reset(s, with_reset);
287+
invalidate_socket_handle();
282288
}
283289

284290
/* 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
@@ -558,7 +558,6 @@ TCP_Transporter::shutdown()
558558
{
559559
DEB_MULTI_TRP(("Close socket for trp %u", getTransporterIndex()));
560560
theSocket.close();
561-
theSocket.invalidate();
562561
}
563562
else
564563
{

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@ bool Transporter::do_disconnect(int err, bool send_source)
195195
DEB_MULTI_TRP(("Close trp_id %u in inactive mode, socket valid",
196196
getTransporterIndex()));
197197
theSocket.close();
198-
theSocket.invalidate();
199198
}
200199
else
201200
{

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

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

177177
log_ssl_error(desc);
178178
close();
179-
invalidate();
180179
return false;
181180
}
182181

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,6 @@ SocketClient::authenticate(NdbSocket & secureSocket)
229229
m_auth->error(r));
230230
}
231231
secureSocket.close();
232-
secureSocket.invalidate();
233232
}
234233
return r;
235234
}

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
@@ -721,7 +721,6 @@ int ndb_mgm_is_connected(NdbMgmHandle handle)
721721
{
722722
handle->connected= 0;
723723
handle->socket.close();
724-
handle->socket.invalidate();
725724
}
726725
}
727726
return handle->connected;
@@ -1062,7 +1061,6 @@ int
10621061
ndb_mgm_disconnect_quiet(NdbMgmHandle handle)
10631062
{
10641063
handle->socket.close();
1065-
handle->socket.invalidate();
10661064
handle->connected = 0;
10671065

10681066
return 0;

storage/ndb/src/mgmsrv/Services.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ MgmApiSession::~MgmApiSession()
413413
if(m_secure_socket.is_valid())
414414
{
415415
m_secure_socket.close();
416-
m_secure_socket.invalidate();
417416
}
418417
if(m_cert)
419418
{
@@ -514,7 +513,6 @@ MgmApiSession::runSession()
514513
if(m_secure_socket.is_valid())
515514
{
516515
m_secure_socket.close();
517-
m_secure_socket.invalidate();
518516
}
519517
NdbMutex_Unlock(m_mutex);
520518

@@ -2037,7 +2035,6 @@ MgmApiSession::transporter_connect(Parser_t::Context &ctx,
20372035
}
20382036
// Close the socket to indicate failure to client
20392037
m_secure_socket.close_with_reset(close_with_reset);
2040-
m_secure_socket.invalidate(); // Already closed
20412038
}
20422039
else
20432040
{

0 commit comments

Comments
 (0)