From 89580524ea794f67877ebb16d34bfdff585727a1 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 28 Mar 2026 01:35:53 +0000 Subject: [PATCH] Fix socket handle leak: always call closesocket after shutdown in disconnect When socket_transport_disconnect was called on a non-binding socket and shutdown() failed, closesocket() was not called, leaking the socket handle and associated Winsock internal allocations (WahInsertHandleContext). Per Windows/POSIX documentation, closesocket()/close() must always be called to release socket resources, regardless of whether shutdown() succeeds or fails. shutdown() is for graceful connection termination while closesocket()/close() is for releasing the socket descriptor. This fix ensures closesocket()/close() is always called after shutdown() is attempted, fixing the memory leaks detected by VLD in the socket_transport_int integration tests. Changes: - win32/src/socket_transport_win32.c: Always call closesocket() - linux/src/socket_transport_linux.c: Always call close() - Updated SRS requirements and unit tests for both platforms Fixes AB#37142411 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../socket_transport_linux_requirements.md | 2 +- linux/src/socket_transport_linux.c | 12 +++++------- .../socket_transport_linux_ut.c | 4 +++- .../socket_transport_win32_requirements.md | 2 +- win32/src/socket_transport_win32.c | 19 ++++++++++--------- .../socket_transport_win32_ut.c | 6 ++++-- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/linux/devdoc/socket_transport_linux_requirements.md b/linux/devdoc/socket_transport_linux_requirements.md index 5fb90865..2d402ac4 100644 --- a/linux/devdoc/socket_transport_linux_requirements.md +++ b/linux/devdoc/socket_transport_linux_requirements.md @@ -183,7 +183,7 @@ MOCKABLE_FUNCTION(, void, socket_transport_disconnect, SOCKET_TRANSPORT_HANDLE, **SRS_SOCKET_TRANSPORT_LINUX_11_025: [** `socket_transport_disconnect` shall call `shutdown` to stop both the transmit and reception of the connected socket. **]** -**SRS_SOCKET_TRANSPORT_LINUX_11_026: [** If `shutdown` does not return 0, the socket is not valid therefore `socket_transport_disconnect` shall not call 'close' **]** +**SRS_SOCKET_TRANSPORT_LINUX_11_026: [** If `shutdown` does not return 0, `socket_transport_disconnect` shall log the error. **]** **SRS_SOCKET_TRANSPORT_LINUX_11_023: [** `socket_transport_disconnect` shall call `close` to disconnect the connected socket. **]** diff --git a/linux/src/socket_transport_linux.c b/linux/src/socket_transport_linux.c index c368001c..7f64d771 100644 --- a/linux/src/socket_transport_linux.c +++ b/linux/src/socket_transport_linux.c @@ -340,16 +340,14 @@ void socket_transport_disconnect(SOCKET_TRANSPORT_HANDLE socket_transport) // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_025: [ socket_transport_disconnect shall call shutdown to stop both the transmit and reception of the connected socket. ] if (shutdown(socket_transport->socket, 2) != 0) { - // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_026: [ If shutdown does not return 0, the socket is not valid therefore socket_transport_disconnect shall not call 'close' ] + // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_026: [ If shutdown does not return 0, socket_transport_disconnect shall log the error. ] LogErrorNo("shutdown failed on socket: %" PRI_SOCKET "", socket_transport->socket); } - else + + // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_023: [ socket_transport_disconnect shall call close to disconnect the connected socket. ] + if (close(socket_transport->socket) != 0) { - // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_023: [ socket_transport_disconnect shall call close to disconnect the connected socket. ] - if (close(socket_transport->socket) != 0) - { - LogErrorNo("Close failure on socket: %" PRI_SOCKET "", socket_transport->socket); - } + LogErrorNo("Close failure on socket: %" PRI_SOCKET "", socket_transport->socket); } // Codes_SRS_SOCKET_TRANSPORT_LINUX_11_024: [ socket_transport_disconnect shall call sm_close_end. ] sm_close_end(socket_transport->sm); diff --git a/linux/tests/socket_transport_linux_ut/socket_transport_linux_ut.c b/linux/tests/socket_transport_linux_ut/socket_transport_linux_ut.c index f7853341..222c85e3 100644 --- a/linux/tests/socket_transport_linux_ut/socket_transport_linux_ut.c +++ b/linux/tests/socket_transport_linux_ut/socket_transport_linux_ut.c @@ -511,7 +511,7 @@ TEST_FUNCTION(socket_transport_disconnect_close_fail_succeed) socket_transport_destroy(socket_handle); } -/*Tests_SRS_SOCKET_TRANSPORT_LINUX_11_026: [ If shutdown does not return 0, the socket is not valid therefore socket_transport_disconnect shall not call 'close' ]*/ +/*Tests_SRS_SOCKET_TRANSPORT_LINUX_11_026: [ If shutdown does not return 0, socket_transport_disconnect shall log the error. ]*/ TEST_FUNCTION(socket_transport_disconnect_shutdown_fail_succeed) { //arrange @@ -529,6 +529,8 @@ TEST_FUNCTION(socket_transport_disconnect_shutdown_fail_succeed) STRICT_EXPECTED_CALL(sm_close_begin(IGNORED_ARG)); STRICT_EXPECTED_CALL(shutdown(IGNORED_ARG, 2)) .SetReturn(MU_FAILURE); + STRICT_EXPECTED_CALL(close(IGNORED_ARG)) + .SetReturn(0); STRICT_EXPECTED_CALL(sm_close_end(IGNORED_ARG)); //act diff --git a/win32/devdoc/socket_transport_win32_requirements.md b/win32/devdoc/socket_transport_win32_requirements.md index 6dbb43d1..0266dd73 100644 --- a/win32/devdoc/socket_transport_win32_requirements.md +++ b/win32/devdoc/socket_transport_win32_requirements.md @@ -209,7 +209,7 @@ MOCKABLE_FUNCTION(, void, socket_transport_disconnect, SOCKET_TRANSPORT_HANDLE, **SRS_SOCKET_TRANSPORT_WIN32_09_029: [** If `sm_close_begin` does not return `SM_EXEC_GRANTED`, `socket_transport_disconnect` shall fail and return. **]** -**SRS_SOCKET_TRANSPORT_WIN32_09_083: [** If `shutdown` does not return 0 on a socket that is not a binding socket, the socket is not valid therefore `socket_transport_disconnect` shall not call `close` **]** +**SRS_SOCKET_TRANSPORT_WIN32_09_083: [** If the type is not `SOCKET_BINDING`, `socket_transport_disconnect` shall call `shutdown`. **]** **SRS_SOCKET_TRANSPORT_WIN32_09_030: [** `socket_transport_disconnect` shall call `closesocket` to disconnect the connected socket. **]** diff --git a/win32/src/socket_transport_win32.c b/win32/src/socket_transport_win32.c index d3cc4c1a..1d326780 100644 --- a/win32/src/socket_transport_win32.c +++ b/win32/src/socket_transport_win32.c @@ -382,19 +382,20 @@ void socket_transport_disconnect(SOCKET_TRANSPORT_HANDLE socket_transport) SM_RESULT close_result = sm_close_begin(socket_transport->sm); if (close_result == SM_EXEC_GRANTED) { - // Codes_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If shutdown does not return 0 on a socket that is not a binding socket, the socket is not valid therefore socket_transport_disconnect shall not call close ] - if (socket_transport->type != SOCKET_BINDING && shutdown(socket_transport->socket, SD_BOTH) != 0) + // Codes_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If the type is not SOCKET_BINDING, socket_transport_disconnect shall call shutdown. ] + if (socket_transport->type != SOCKET_BINDING) { - LogLastError("shutdown failed on socket: %" PRI_SOCKET "", socket_transport->socket); - } - else - { - // Codes_SRS_SOCKET_TRANSPORT_WIN32_09_030: [ socket_transport_disconnect shall call closesocket to disconnect the connected socket. ] - if (closesocket(socket_transport->socket) != 0) + if (shutdown(socket_transport->socket, SD_BOTH) != 0) { - LogLastError("Failure in closesocket %" PRI_SOCKET "", socket_transport->socket); + LogLastError("shutdown failed on socket: %" PRI_SOCKET "", socket_transport->socket); } } + + // Codes_SRS_SOCKET_TRANSPORT_WIN32_09_030: [ socket_transport_disconnect shall call closesocket to disconnect the connected socket. ] + if (closesocket(socket_transport->socket) != 0) + { + LogLastError("Failure in closesocket %" PRI_SOCKET "", socket_transport->socket); + } // Codes_SRS_SOCKET_TRANSPORT_WIN32_09_031: [ socket_transport_disconnect shall call sm_close_end. ] sm_close_end(socket_transport->sm); } diff --git a/win32/tests/socket_transport_win32_ut/socket_transport_win32_ut.c b/win32/tests/socket_transport_win32_ut/socket_transport_win32_ut.c index 56fb04f8..ad2db53e 100644 --- a/win32/tests/socket_transport_win32_ut/socket_transport_win32_ut.c +++ b/win32/tests/socket_transport_win32_ut/socket_transport_win32_ut.c @@ -674,7 +674,7 @@ TEST_FUNCTION(socket_transport_disconnect_invalid_arguments) } -/*Tests_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If shutdown does not return 0 on a socket that is not a binding socket, the socket is not valid therefore socket_transport_disconnect shall not call close]*/ +/*Tests_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If the type is not SOCKET_BINDING, socket_transport_disconnect shall call shutdown. ]*/ TEST_FUNCTION(socket_transport_disconnect_shutdown_fail) { //arrange @@ -692,6 +692,8 @@ TEST_FUNCTION(socket_transport_disconnect_shutdown_fail) STRICT_EXPECTED_CALL(sm_close_begin(IGNORED_ARG)); STRICT_EXPECTED_CALL(shutdown(IGNORED_ARG, 2)) .SetReturn(MU_FAILURE); + STRICT_EXPECTED_CALL(closesocket(IGNORED_ARG)) + .SetReturn(0); STRICT_EXPECTED_CALL(sm_close_end(IGNORED_ARG)); //act @@ -736,7 +738,7 @@ TEST_FUNCTION(socket_transport_disconnect_failure_closesocket) socket_transport_destroy(socket_handle); } -/*Tests_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If shutdown does not return 0 on a socket that is not a binding socket, the socket is not valid therefore socket_transport_disconnect shall not call close ]*/ +/*Tests_SRS_SOCKET_TRANSPORT_WIN32_09_083: [ If the type is not SOCKET_BINDING, socket_transport_disconnect shall call shutdown. ]*/ TEST_FUNCTION(socket_transport_disconnect_binding_socket_closesocket) { //arrange