From 151bb05cd61987723f8de9f0c7ac71b4b5430307 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Thu, 22 Nov 2018 18:00:26 +0100 Subject: [socket-nif] Fixed two minor problems with socket close and down * Socket close callback Only demonitor the closer if its local (close) and its not a direct call. There was a spurious warning message. * Down callback Only process this event if the socket is not closed or closing. OTP-14831 --- erts/emulator/nifs/common/socket_nif.c | 279 +++++++++++++++++++-------------- 1 file changed, 157 insertions(+), 122 deletions(-) (limited to 'erts/emulator/nifs') diff --git a/erts/emulator/nifs/common/socket_nif.c b/erts/emulator/nifs/common/socket_nif.c index cc11bb159f..ee1aa61c35 100644 --- a/erts/emulator/nifs/common/socket_nif.c +++ b/erts/emulator/nifs/common/socket_nif.c @@ -775,7 +775,7 @@ typedef struct { ErlNifEnv* env; /* +++ Controller (owner) process +++ */ - ErlNifPid ctrlPid; + ErlNifPid ctrlPid; // ErlNifMonitor ctrlMon; ESockMonitor ctrlMon; @@ -5232,7 +5232,6 @@ ERL_NIF_TERM nsend(ErlNifEnv* env, else save_errno = -1; // The value does not actually matter in this case - return send_check_result(env, descP, written, sndDataP->size, save_errno, sendRef); @@ -6185,8 +6184,10 @@ ERL_NIF_TERM nclose(ErlNifEnv* env, int type = descP->type; int protocol = descP->protocol; - SSDBG( descP, ("SOCKET", "nclose -> [%d] entry (0x%lX, 0x%lX, 0x%lX)\r\n", + SSDBG( descP, ("SOCKET", + "nclose -> [%d] entry (0x%lX, 0x%lX, 0x%lX, 0x%lX)\r\n", descP->sock, + descP->state, descP->currentWriterP, descP->currentReaderP, descP->currentAcceptorP) ); @@ -6246,7 +6247,6 @@ ERL_NIF_TERM nclose(ErlNifEnv* env, SSDBG( descP, ("SOCKET", "nclose -> [%d] stop was called\r\n", descP->sock) ); dec_socket(domain, type, protocol); - DEMONP("nclose -> closer", env, descP, &descP->closerMon); reply = esock_atom_ok; } else if (selectRes & ERL_NIF_SELECT_STOP_SCHEDULED) { /* The stop callback function has been *scheduled* which means that we @@ -6284,8 +6284,9 @@ ERL_NIF_TERM nclose(ErlNifEnv* env, SSDBG( descP, ("SOCKET", "nclose -> [%d] done when: " + "\r\n state: 0x%lX" "\r\n reply: %T" - "\r\n", descP->sock, reply) ); + "\r\n", descP->sock, descP->state, reply) ); return reply; } @@ -16901,8 +16902,29 @@ void socket_stop(ErlNifEnv* env, void* obj, int fd, int is_direct_call) MLOCK(descP->accMtx); MLOCK(descP->closeMtx); - SSDBG( descP, ("SOCKET", "socket_stop -> all mutex(s) locked\r\n") ); - + SSDBG( descP, ("SOCKET", "socket_stop -> " + "[%d, %T] all mutex(s) locked when counters:" + "\r\n writePkgCnt: %u" + "\r\n writeByteCnt: %u" + "\r\n writeTries: %u" + "\r\n writeWaits: %u" + "\r\n writeFails: %u" + "\r\n readPkgCnt: %u" + "\r\n readByteCnt: %u" + "\r\n readTries: %u" + "\r\n readWaits: %u" + "\r\n", + descP->sock, descP->ctrlPid, + descP->writePkgCnt, + descP->writeByteCnt, + descP->writeTries, + descP->writeWaits, + descP->writeFails, + descP->readPkgCnt, + descP->readByteCnt, + descP->readTries, + descP->readWaits) ); + descP->state = SOCKET_STATE_CLOSING; // Just in case...??? descP->isReadable = FALSE; descP->isWritable = FALSE; @@ -17044,7 +17066,8 @@ void socket_stop(ErlNifEnv* env, void* obj, int fd, int is_direct_call) * */ - if (descP->closeLocal) { + if (descP->closeLocal && + !is_direct_call) { /* +++ send close message to the waiting process +++ * @@ -17054,13 +17077,18 @@ void socket_stop(ErlNifEnv* env, void* obj, int fd, int is_direct_call) * * WHAT HAPPENS IF THE RECEIVER HAS DIED IN THE MEANTIME???? * + * Also, we should really *always* use a tag unique to this + * (nif-) module. Some like (in this case): + * + * {'$socket', close, CloseRef} + * * */ - send_msg(env, MKT2(env, atom_close, descP->closeRef), &descP->closerPid); + send_msg(env, + MKT2(env, atom_close, descP->closeRef), &descP->closerPid); - DEMONP("socket_stop -> closer", - env, descP, &descP->closerMon); + DEMONP("socket_stop -> closer", env, descP, &descP->closerMon); } else { @@ -17199,137 +17227,144 @@ void socket_down(ErlNifEnv* env, SSDBG( descP, ("SOCKET", "socket_down -> OTHER mon\r\n") ); } */ - - if (compare_pids(env, &descP->ctrlPid, pid)) { - int selectRes; - - /* We don't bother with the queue cleanup here - - * we leave it to the stop callback function. - */ - - SSDBG( descP, - ("SOCKET", "socket_down -> controlling process exit\r\n") ); - descP->state = SOCKET_STATE_CLOSING; - descP->closeLocal = TRUE; - descP->closerPid = *pid; - descP->closerMon = (ESockMonitor) *mon; - descP->closeRef = MKREF(env); // Do we really need this in this case? - - /* - esock_dbg_printf("DOWN", - "[%d] select stop %u,%u,%u,%d\r\n", - descP->sock, - monP->raw[0], monP->raw[1], - monP->raw[2], monP->raw[3]); - */ + if (!IS_CLOSED(descP)) { + if (compare_pids(env, &descP->ctrlPid, pid)) { + int selectRes; - selectRes = enif_select(env, descP->sock, (ERL_NIF_SELECT_STOP), - descP, NULL, descP->closeRef); + /* We don't bother with the queue cleanup here - + * we leave it to the stop callback function. + */ - if (selectRes & ERL_NIF_SELECT_STOP_CALLED) { - /* We are done - wwe can finalize (socket close) directly */ SSDBG( descP, - ("SOCKET", "socket_down -> [%d] stop called\r\n", descP->sock) ); - dec_socket(descP->domain, descP->type, descP->protocol); - descP->state = SOCKET_STATE_CLOSED; - - /* And finally close the socket. - * Since we close the socket because of an exiting owner, - * we do not need to wait for buffers to sync (linger). - * If the owner wish to ensure the buffer are written, - * it should have closed teh socket explicitly... - */ - if (sock_close(descP->sock) != 0) { - int save_errno = sock_errno(); - - esock_warning_msg("Failed closing socket for terminating " - "controlling process: " - "\r\n Controlling Process: %T" - "\r\n Descriptor: %d" - "\r\n Errno: %d" - "\r\n", pid, descP->sock, save_errno); - } - sock_close_event(descP->event); + ("SOCKET", "socket_down -> controlling process exit\r\n") ); + + descP->state = SOCKET_STATE_CLOSING; + descP->closeLocal = TRUE; + descP->closerPid = *pid; + descP->closerMon = (ESockMonitor) *mon; + descP->closeRef = MKREF(env); // Do we really need this in this case? - descP->sock = INVALID_SOCKET; - descP->event = INVALID_EVENT; + /* + esock_dbg_printf("DOWN", + "[%d] select stop %u,%u,%u,%d\r\n", + descP->sock, + monP->raw[0], monP->raw[1], + monP->raw[2], monP->raw[3]); + */ - descP->state = SOCKET_STATE_CLOSED; + selectRes = enif_select(env, descP->sock, (ERL_NIF_SELECT_STOP), + descP, NULL, descP->closeRef); + + if (selectRes & ERL_NIF_SELECT_STOP_CALLED) { + /* We are done - we can finalize (socket close) directly */ + SSDBG( descP, + ("SOCKET", + "socket_down -> [%d] stop called\r\n", descP->sock) ); + dec_socket(descP->domain, descP->type, descP->protocol); + descP->state = SOCKET_STATE_CLOSED; + + /* And finally close the socket. + * Since we close the socket because of an exiting owner, + * we do not need to wait for buffers to sync (linger). + * If the owner wish to ensure the buffer are written, + * it should have closed the socket explicitly... + */ + if (sock_close(descP->sock) != 0) { + int save_errno = sock_errno(); + + esock_warning_msg("Failed closing socket for terminating " + "controlling process: " + "\r\n Controlling Process: %T" + "\r\n Descriptor: %d" + "\r\n Errno: %d" + "\r\n", pid, descP->sock, save_errno); + } + sock_close_event(descP->event); - } else if (selectRes & ERL_NIF_SELECT_STOP_SCHEDULED) { - /* The stop callback function has been *scheduled* which means that - * "should" wait for it to complete. But since we are in a callback - * (down) function, we cannot... - * So, we must close the socket - */ - SSDBG( descP, - ("SOCKET", - "socket_down -> [%d] stop scheduled\r\n", descP->sock) ); - dec_socket(descP->domain, descP->type, descP->protocol); + descP->sock = INVALID_SOCKET; + descP->event = INVALID_EVENT; - /* And now what? We can't wait for the stop function here... - * So, we simply close it here and leave the rest of the "close" - * for later (when the stop function actually gets called... - */ + descP->state = SOCKET_STATE_CLOSED; - if (sock_close(descP->sock) != 0) { - int save_errno = sock_errno(); - - esock_warning_msg("Failed closing socket for terminating " - "controlling process: " + } else if (selectRes & ERL_NIF_SELECT_STOP_SCHEDULED) { + /* The stop callback function has been *scheduled* which means + * that "should" wait for it to complete. But since we are in + * a callback (down) function, we cannot... + * So, we must close the socket + */ + SSDBG( descP, + ("SOCKET", + "socket_down -> [%d] stop scheduled\r\n", + descP->sock) ); + dec_socket(descP->domain, descP->type, descP->protocol); + + /* And now what? We can't wait for the stop function here... + * So, we simply close it here and leave the rest of the "close" + * for later (when the stop function actually gets called... + */ + + if (sock_close(descP->sock) != 0) { + int save_errno = sock_errno(); + + esock_warning_msg("Failed closing socket for terminating " + "controlling process: " + "\r\n Controlling Process: %T" + "\r\n Descriptor: %d" + "\r\n Errno: %d" + "\r\n", pid, descP->sock, save_errno); + } + sock_close_event(descP->event); + + } else { + + /* + * + * + * WE SHOULD REALLY HAVE A WAY TO CLOBBER THE SOCKET, + * SO WE DON'T LET STUFF LEAK. + * NOW, BECAUSE WE FAILED TO SELECT, WE CANNOT FINISH + * THE CLOSE, WHAT TO DO? ABORT? + * + * + */ + esock_warning_msg("Failed selecting stop when handling down " + "of controlling process: " + "\r\n Select Res: %d" "\r\n Controlling Process: %T" "\r\n Descriptor: %d" - "\r\n Errno: %d" - "\r\n", pid, descP->sock, save_errno); + "\r\n Monitor: %u.%u.%u.%u" + "\r\n", selectRes, pid, descP->sock, + monP->raw[0], monP->raw[1], + monP->raw[2], monP->raw[3]); } - sock_close_event(descP->event); - + } else { - /* - * + /* check all operation queue(s): acceptor, writer and reader. * - * WE SHOULD REALLY HAVE A WAY TO CLOBBER THE SOCKET, - * SO WE DON'T LET STUFF LEAK. - * NOW, BECAUSE WE FAILED TO SELECT, WE CANNOT FINISH - * THE CLOSE, WHAT TO DO? ABORT? + * Is it really any point in doing this if the socket is closed? * - * */ - esock_warning_msg("Failed selecting stop when handling down " - "of controlling process: " - "\r\n Select Res: %d" - "\r\n Controlling Process: %T" - "\r\n Descriptor: %d" - "\r\n Monitor: %u.%u.%u.%u" - "\r\n", selectRes, pid, descP->sock, - monP->raw[0], monP->raw[1], - monP->raw[2], monP->raw[3]); - } - - } else { - - /* check all operation queue(s): acceptor, writer and reader. */ - - SSDBG( descP, ("SOCKET", "socket_down -> other process term\r\n") ); + SSDBG( descP, ("SOCKET", "socket_down -> other process term\r\n") ); - MLOCK(descP->accMtx); - if (descP->currentAcceptorP != NULL) - socket_down_acceptor(env, descP, pid); - MUNLOCK(descP->accMtx); - - MLOCK(descP->writeMtx); - if (descP->currentWriterP != NULL) - socket_down_writer(env, descP, pid); - MUNLOCK(descP->writeMtx); - - MLOCK(descP->readMtx); - if (descP->currentReaderP != NULL) - socket_down_reader(env, descP, pid); - MUNLOCK(descP->readMtx); + MLOCK(descP->accMtx); + if (descP->currentAcceptorP != NULL) + socket_down_acceptor(env, descP, pid); + MUNLOCK(descP->accMtx); + + MLOCK(descP->writeMtx); + if (descP->currentWriterP != NULL) + socket_down_writer(env, descP, pid); + MUNLOCK(descP->writeMtx); + MLOCK(descP->readMtx); + if (descP->currentReaderP != NULL) + socket_down_reader(env, descP, pid); + MUNLOCK(descP->readMtx); + + } } /* -- cgit v1.2.3