Skip to content

Commit 33a185c

Browse files
frazerclementfrazerclement
committed
Bug#36623058 Ndb : Shared memory transporter connection setup error paths not closing socket
In SHM_Transporter specific connect_client and connect_server overrides, make sure to close socket, and de-initialise buffers on error paths prior to successful connection setup. This avoids leaking connected sockets, or attempting to double-initialise buffers in cases where some step of the SHM_Transporter setup fails. Change-Id: Ie78b7ce302fffb280b9cd5f287a1c82ca3b64f45
1 parent 97fdbda commit 33a185c

File tree

1 file changed

+166
-147
lines changed

1 file changed

+166
-147
lines changed

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

Lines changed: 166 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -289,98 +289,115 @@ SHM_Transporter::connect_server_impl(NDB_SOCKET_TYPE sockfd)
289289
SocketOutputStream s_output(sockfd);
290290
SocketInputStream s_input(sockfd);
291291

292-
// Create
293-
if (!_shmSegCreated)
294-
{
295-
if (!ndb_shm_create())
292+
do // Close socket on errors
293+
{
294+
// Create
295+
if (!_shmSegCreated) {
296+
if (!ndb_shm_create()) {
297+
DEBUG_FPRINTF((stderr,
298+
"(%u)connect_server_impl failed LINE:%d,"
299+
" to remote node %d\n",
300+
localNodeId, __LINE__, remoteNodeId));
301+
break;
302+
}
303+
_shmSegCreated = true;
304+
DEBUG_FPRINTF(
305+
(stderr, "(%u)ndb_shm_create()(%u)\n", localNodeId, remoteNodeId));
306+
}
307+
308+
// Attach
309+
if (!_attached) {
310+
if (!ndb_shm_attach()) {
311+
DEBUG_FPRINTF((stderr,
312+
"(%u)connect_server_impl failed LINE:%d,"
313+
" to remote node %d\n",
314+
localNodeId, __LINE__, remoteNodeId));
315+
break;
316+
}
317+
_attached = true;
318+
DEBUG_FPRINTF(
319+
(stderr, "(%u)ndb_shm_attach()(%u)\n", localNodeId, remoteNodeId));
320+
}
321+
322+
require(!setupBuffersDone);
296323
{
324+
DEBUG_FPRINTF((stderr, "(%u)setupBuffers(%u) Line:%d\n", localNodeId,
325+
remoteNodeId, __LINE__));
326+
if (setupBuffers()) {
327+
g_eventLogger->info("Shared memory not supported on this platform");
328+
detach_shm(false);
329+
break;
330+
}
331+
setupBuffersDone = true;
332+
}
333+
334+
// Send ok to client
335+
s_output.println("shm server 1 ok: %d",
336+
m_transporter_registry.m_shm_own_pid);
337+
338+
// Wait for ok from client
339+
char buf[256];
340+
DBUG_PRINT("info", ("Wait for ok from client"));
341+
if (s_input.gets(buf, sizeof(buf)) == NULL) {
297342
DEBUG_FPRINTF((stderr, "(%u)connect_server_impl failed LINE:%d,"
298343
" to remote node %d\n",
299344
localNodeId, __LINE__, remoteNodeId));
300-
DBUG_RETURN(false);
345+
setupBuffersUndone();
346+
detach_shm(false);
347+
break;
301348
}
302-
_shmSegCreated = true;
303-
DEBUG_FPRINTF((stderr, "(%u)ndb_shm_create()(%u)\n",
304-
localNodeId, remoteNodeId));
305-
}
306349

307-
// Attach
308-
if (!_attached)
309-
{
310-
if (!ndb_shm_attach())
311-
{
350+
if (sscanf(buf, "shm client 1 ok: %d", &m_remote_pid) != 1) {
312351
DEBUG_FPRINTF((stderr, "(%u)connect_server_impl failed LINE:%d,"
313352
" to remote node %d\n",
314353
localNodeId, __LINE__, remoteNodeId));
315-
DBUG_RETURN(false);
354+
setupBuffersUndone();
355+
detach_shm(false);
356+
break;
316357
}
317-
_attached = true;
318-
DEBUG_FPRINTF((stderr, "(%u)ndb_shm_attach()(%u)\n",
319-
localNodeId, remoteNodeId));
320-
}
321358

322-
require(!setupBuffersDone);
323-
{
324-
DEBUG_FPRINTF((stderr, "(%u)setupBuffers(%u) Line:%d\n",
325-
localNodeId, remoteNodeId, __LINE__));
326-
if (setupBuffers())
327-
{
328-
fprintf(stderr, "Shared memory not supported on this platform\n");
359+
DEBUG_FPRINTF(
360+
(stderr, "(%u)connect_common()(%u)\n", localNodeId, remoteNodeId));
361+
if (!connect_common(sockfd)) {
362+
DEBUG_FPRINTF((stderr,
363+
"(%u)connect_server_impl failed LINE:%d,"
364+
" to remote node %d\n",
365+
localNodeId, __LINE__, remoteNodeId));
366+
setupBuffersUndone();
329367
detach_shm(false);
330-
DBUG_RETURN(false);
368+
break;
331369
}
332-
setupBuffersDone=true;
333-
}
334-
335-
// Send ok to client
336-
s_output.println("shm server 1 ok: %d",
337-
m_transporter_registry.m_shm_own_pid);
338-
339-
// Wait for ok from client
340-
char buf[256];
341-
DBUG_PRINT("info", ("Wait for ok from client"));
342-
if (s_input.gets(buf, sizeof(buf)) == 0)
343-
{
344-
DEBUG_FPRINTF((stderr, "(%u)connect_server_impl failed LINE:%d,"
345-
" to remote node %d\n",
346-
localNodeId, __LINE__, remoteNodeId));
347-
detach_shm(false);
348-
DBUG_RETURN(false);
349-
}
350-
351-
if (sscanf(buf, "shm client 1 ok: %d", &m_remote_pid) != 1)
352-
{
353-
DEBUG_FPRINTF((stderr, "(%u)connect_server_impl failed LINE:%d,"
354-
" to remote node %d\n",
355-
localNodeId, __LINE__, remoteNodeId));
356-
detach_shm(false);
357-
DBUG_RETURN(false);
358-
}
359-
360-
DEBUG_FPRINTF((stderr, "(%u)connect_common()(%u)\n",
361-
localNodeId, remoteNodeId));
362-
int r= connect_common(sockfd);
363370

364-
if (r)
365-
{
366371
// Send ok to client
367372
s_output.println("shm server 2 ok");
368373
// Wait for ok from client
369-
if (s_input.gets(buf, 256) == 0)
370-
{
374+
if (s_input.gets(buf, 256) == 0) {
371375
DEBUG_FPRINTF((stderr, "(%u)connect_server_impl failed LINE:%d,"
372376
" to remote node %d\n",
373377
localNodeId, __LINE__, remoteNodeId));
378+
setupBuffersUndone();
374379
detach_shm(false);
375-
DBUG_RETURN(false);
380+
break;
376381
}
382+
377383
DBUG_PRINT("info", ("Successfully connected server to node %d",
378384
remoteNodeId));
379-
}
380-
DEBUG_FPRINTF((stderr, "(%u)set_socket()(%u)\n",
381-
localNodeId, remoteNodeId));
382-
set_socket(sockfd);
383-
DBUG_RETURN(r);
385+
386+
DEBUG_FPRINTF(
387+
(stderr, "(%u)set_socket()(%u)\n", localNodeId, remoteNodeId));
388+
set_socket(sockfd);
389+
DBUG_RETURN(true);
390+
} while (0);
391+
392+
/* Error occurred */
393+
394+
/* Existing error handling should have cleaned up for retry */
395+
assert(!setupBuffersDone);
396+
assert(!_attached);
397+
assert(!_shmSegCreated);
398+
399+
my_socket_close(sockfd);
400+
DBUG_RETURN(false);
384401
}
385402

386403
void
@@ -405,115 +422,117 @@ SHM_Transporter::connect_client_impl(NDB_SOCKET_TYPE sockfd)
405422
SocketOutputStream s_output(sockfd);
406423
char buf[256];
407424

408-
// Wait for server to create and attach
409-
DBUG_PRINT("info", ("Wait for server to create and attach"));
410-
if (s_input.gets(buf, 256) == 0)
411-
{
412-
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
413-
" to remote node %d\n",
414-
localNodeId, __LINE__, remoteNodeId));
415-
416-
DBUG_PRINT("error", ("Server id %d did not attach",
417-
remoteNodeId));
418-
DBUG_RETURN(false);
419-
}
420-
421-
if(sscanf(buf, "shm server 1 ok: %d", &m_remote_pid) != 1)
422-
{
423-
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
424-
" to remote node %d\n",
425-
localNodeId, __LINE__, remoteNodeId));
426-
DBUG_RETURN(false);
427-
}
428-
429-
// Create
430-
if(!_shmSegCreated)
425+
do // Close socket on errors
431426
{
432-
if (!ndb_shm_get())
433-
{
427+
// Wait for server to create and attach
428+
DBUG_PRINT("info", ("Wait for server to create and attach"));
429+
if (s_input.gets(buf, 256) == NULL) {
434430
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
435431
" to remote node %d\n",
436432
localNodeId, __LINE__, remoteNodeId));
437-
DBUG_PRINT("error", ("Failed create of shm seg to node %d",
438-
remoteNodeId));
439-
DBUG_RETURN(false);
433+
434+
DBUG_PRINT("error", ("Server id %d did not attach", remoteNodeId));
435+
break;
440436
}
441-
_shmSegCreated = true;
442-
DEBUG_FPRINTF((stderr, "(%u)ndb_shm_get(%u)\n",
443-
localNodeId, remoteNodeId));
444-
}
445437

446-
// Attach
447-
if (!_attached)
448-
{
449-
if (!ndb_shm_attach())
450-
{
438+
if (sscanf(buf, "shm server 1 ok: %d", &m_remote_pid) != 1) {
451439
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
452440
" to remote node %d\n",
453441
localNodeId, __LINE__, remoteNodeId));
454-
DBUG_PRINT("error", ("Failed attach of shm seg to node %d",
455-
remoteNodeId));
456-
DBUG_RETURN(false);
442+
break;
457443
}
458-
_attached = true;
459-
DEBUG_FPRINTF((stderr, "(%u)ndb_shm_attach(%u)\n",
460-
localNodeId, remoteNodeId));
461-
}
462444

463-
require(!setupBuffersDone);
464-
{
465-
DEBUG_FPRINTF((stderr, "(%u)setupBuffers(%u) Line:%d\n",
466-
localNodeId, remoteNodeId, __LINE__));
467-
if (setupBuffers())
468-
{
469-
fprintf(stderr, "Shared memory not supported on this platform\n");
470-
detach_shm(false);
471-
DBUG_RETURN(false);
445+
// Create
446+
if (!_shmSegCreated) {
447+
if (!ndb_shm_get()) {
448+
DEBUG_FPRINTF((stderr,
449+
"(%u)connect_client_impl failed LINE:%d,"
450+
" to remote node %d\n",
451+
localNodeId, __LINE__, remoteNodeId));
452+
DBUG_PRINT("error",
453+
("Failed create of shm seg to node %d", remoteNodeId));
454+
break;
455+
}
456+
_shmSegCreated = true;
457+
DEBUG_FPRINTF(
458+
(stderr, "(%u)ndb_shm_get(%u)\n", localNodeId, remoteNodeId));
459+
}
460+
461+
// Attach
462+
if (!_attached) {
463+
if (!ndb_shm_attach()) {
464+
DEBUG_FPRINTF((stderr,
465+
"(%u)connect_client_impl failed LINE:%d,"
466+
" to remote node %d\n",
467+
localNodeId, __LINE__, remoteNodeId));
468+
DBUG_PRINT("error",
469+
("Failed attach of shm seg to node %d", remoteNodeId));
470+
break;
471+
}
472+
_attached = true;
473+
DEBUG_FPRINTF(
474+
(stderr, "(%u)ndb_shm_attach(%u)\n", localNodeId, remoteNodeId));
472475
}
473-
else
476+
477+
require(!setupBuffersDone);
474478
{
479+
DEBUG_FPRINTF((stderr, "(%u)setupBuffers(%u) Line:%d\n", localNodeId,
480+
remoteNodeId, __LINE__));
481+
if (setupBuffers()) {
482+
g_eventLogger->info("Shared memory not supported on this platform");
483+
detach_shm(false);
484+
break;
485+
}
475486
setupBuffersDone=true;
476487
}
477-
}
478488

479-
// Send ok to server
480-
s_output.println("shm client 1 ok: %d",
481-
m_transporter_registry.m_shm_own_pid);
482-
483-
DEBUG_FPRINTF((stderr, "(%u)connect_common(%u)\n",
484-
localNodeId, remoteNodeId));
485-
int r = connect_common(sockfd);
489+
// Send ok to server
490+
s_output.println("shm client 1 ok: %d",
491+
m_transporter_registry.m_shm_own_pid);
486492

487-
if (r)
488-
{
493+
DEBUG_FPRINTF(
494+
(stderr, "(%u)connect_common(%u)\n", localNodeId, remoteNodeId));
495+
if (!connect_common(sockfd)) {
496+
DEBUG_FPRINTF((stderr,
497+
"(%u)connect_client_impl failed LINE:%d,"
498+
" to remote node %d\n",
499+
localNodeId, __LINE__, remoteNodeId));
500+
setupBuffersUndone();
501+
detach_shm(false);
502+
break;
503+
}
504+
489505
// Wait for ok from server
490506
DBUG_PRINT("info", ("Wait for ok from server"));
491-
if (s_input.gets(buf, 256) == 0)
492-
{
507+
if (s_input.gets(buf, 256) == 0) {
493508
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
494509
" to remote node %d\n",
495510
localNodeId, __LINE__, remoteNodeId));
496511
DBUG_PRINT("error", ("No ok from server node %d",
497512
remoteNodeId));
513+
setupBuffersUndone();
498514
detach_shm(false);
499-
DBUG_RETURN(false);
515+
break;
500516
}
501517
// Send ok to server
502518
s_output.println("shm client 2 ok");
503519
DBUG_PRINT("info", ("Successfully connected client to node %d",
504520
remoteNodeId));
505-
}
506-
else
507-
{
508-
DEBUG_FPRINTF((stderr, "(%u)connect_client_impl failed LINE:%d,"
509-
" to remote node %d\n",
510-
localNodeId, __LINE__, remoteNodeId));
511-
detach_shm(false);
512-
}
513-
set_socket(sockfd);
514-
DEBUG_FPRINTF((stderr, "(%u)set_socket(%u)\n",
515-
localNodeId, remoteNodeId));
516-
DBUG_RETURN(r);
521+
522+
set_socket(sockfd);
523+
DEBUG_FPRINTF((stderr, "(%u)set_socket(%u)\n", localNodeId, remoteNodeId));
524+
DBUG_RETURN(true);
525+
} while (0);
526+
527+
/* Error occurred */
528+
529+
/* Existing error handling should have cleaned up for retry */
530+
assert(!setupBuffersDone);
531+
assert(!_attached);
532+
assert(!_shmSegCreated);
533+
534+
my_socket_close(sockfd);
535+
DBUG_RETURN(false);
517536
}
518537

519538
bool

0 commit comments

Comments
 (0)