Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ws2_32/socket.c | 99 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 22 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index b7a570043cd..44214630296 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -267,26 +267,22 @@ static int WS2_recv_base( SOCKET s, LPWSABUF lpBuffers, DWORD dwBufferCount, LPWSAOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine, LPWSABUF lpControlBuffer );
-/* critical section to protect some non-reentrant net function */ -static CRITICAL_SECTION csWSgetXXXbyYYY; -static CRITICAL_SECTION_DEBUG critsect_debug = -{ - 0, 0, &csWSgetXXXbyYYY, - { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList }, - 0, 0, { (DWORD_PTR)(__FILE__ ": csWSgetXXXbyYYY") } -}; -static CRITICAL_SECTION csWSgetXXXbyYYY = { &critsect_debug, -1, 0, 0, 0, 0 }; +#define DECLARE_CRITICAL_SECTION(cs) \ + static CRITICAL_SECTION cs; \ + static CRITICAL_SECTION_DEBUG cs##_debug = \ + { 0, 0, &cs, { &cs##_debug.ProcessLocksList, &cs##_debug.ProcessLocksList }, \ + 0, 0, { (DWORD_PTR)(__FILE__ ": " # cs) }}; \ + static CRITICAL_SECTION cs = { &cs##_debug, -1, 0, 0, 0, 0 } + +DECLARE_CRITICAL_SECTION(csWSgetXXXbyYYY); +DECLARE_CRITICAL_SECTION(cs_if_addr_cache); +DECLARE_CRITICAL_SECTION(cs_socket_list);
static in_addr_t *if_addr_cache; static unsigned int if_addr_cache_size; -static CRITICAL_SECTION cs_if_addr_cache; -static CRITICAL_SECTION_DEBUG cs_if_addr_cache_debug = -{ - 0, 0, &cs_if_addr_cache, - { &cs_if_addr_cache_debug.ProcessLocksList, &cs_if_addr_cache_debug.ProcessLocksList }, - 0, 0, { (DWORD_PTR)(__FILE__ ": cs_if_addr_cache") } -}; -static CRITICAL_SECTION cs_if_addr_cache = { &cs_if_addr_cache_debug, -1, 0, 0, 0, 0 }; + +static SOCKET *socket_list; +static unsigned int socket_list_size;
union generic_unix_sockaddr { @@ -481,6 +477,48 @@ static inline const char *debugstr_optval(const char *optval, int optlenval) #define SOCKET2HANDLE(s) ((HANDLE)(s)) #define HANDLE2SOCKET(h) ((SOCKET)(h))
+static BOOL socket_list_add(SOCKET socket) +{ + unsigned int i, new_size; + SOCKET *new_array; + + EnterCriticalSection(&cs_socket_list); + for (i = 0; i < socket_list_size; ++i) + { + if (!socket_list[i]) + { + socket_list[i] = socket; + LeaveCriticalSection(&cs_socket_list); + return TRUE; + } + } + new_size = max(socket_list_size * 2, 8); + if (!(new_array = heap_realloc(socket_list, new_size * sizeof(*socket_list)))) + return FALSE; + socket_list = new_array; + memset(socket_list + socket_list_size, 0, (new_size - socket_list_size) * sizeof(*socket_list)); + socket_list[socket_list_size] = socket; + socket_list_size = new_size; + LeaveCriticalSection(&cs_socket_list); + return TRUE; +} + +static void socket_list_remove(SOCKET socket) +{ + unsigned int i; + + EnterCriticalSection(&cs_socket_list); + for (i = 0; i < socket_list_size; ++i) + { + if (socket_list[i] == socket) + { + socket_list[i] = 0; + break; + } + } + LeaveCriticalSection(&cs_socket_list); +} + /**************************************************************** * Async IO declarations ****************************************************************/ @@ -2832,6 +2870,11 @@ SOCKET WINAPI WS_accept(SOCKET s, struct WS_sockaddr *addr, int *addrlen32) SERVER_END_REQ; if (!err) { + if (!socket_list_add(as)) + { + CloseHandle(SOCKET2HANDLE(as)); + return SOCKET_ERROR; + } if (addr && addrlen32 && WS_getpeername(as, addr, addrlen32)) { WS_closesocket(as); @@ -3483,6 +3526,7 @@ int WINAPI WS_closesocket(SOCKET s) if (fd >= 0) { release_sock_fd(s, fd); + socket_list_remove(s); if (CloseHandle(SOCKET2HANDLE(s))) res = 0; } @@ -7558,10 +7602,16 @@ SOCKET WINAPI WSASocketW(int af, int type, int protocol, }
/* hack for WSADuplicateSocket */ - if (lpProtocolInfo && lpProtocolInfo->dwServiceFlags4 == 0xff00ff00) { - ret = lpProtocolInfo->dwServiceFlags3; - TRACE("\tgot duplicate %04lx\n", ret); - return ret; + if (lpProtocolInfo && lpProtocolInfo->dwServiceFlags4 == 0xff00ff00) + { + ret = lpProtocolInfo->dwServiceFlags3; + TRACE("\tgot duplicate %04lx\n", ret); + if (!socket_list_add(ret)) + { + CloseHandle(SOCKET2HANDLE(ret)); + return INVALID_SOCKET; + } + return ret; }
if (lpProtocolInfo) @@ -7685,7 +7735,12 @@ SOCKET WINAPI WSASocketW(int af, int type, int protocol, } } #endif - return ret; + if (!socket_list_add(ret)) + { + CloseHandle(SOCKET2HANDLE(ret)); + return INVALID_SOCKET; + } + return ret; }
if (err == WSAEACCES) /* raw socket denied */
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=18670 Signed-off-by: Zebediah Figura z.figura12@gmail.com --- When patches for this issue were sent back in 2014-15, it was assumed that this approach was insufficient, since it wouldn't close duplicated handles. Whether native ws2_32 closes duplicated handles seems never to have been tested, however, and manual testing shows that it does not.
In greater detail:
* It is known, in the first place, that socket handles are NT kernel handles, since they can be passed to CreateIoCompletionPort().
* When calling WSADuplicateSocketW(), a valid handle is stored in the "dwProviderReserved" member of WSAPROTOCOL_INFO. This same handle is returned in a subsequent call to WSASocket() with the same WSAPROTOCOL_INFO structure. Even before WSASocket is called, the handle can be successfully passed to Nt* functions without returning STATUS_INVALID_HANDLE. [Amusingly, our "hack" actually had the right idea, but used the wrong member.]
* If WSASocket() is called on the handle, it is closed by WSACleanup() [i.e. a subsequent NtClose() returns STATUS_INVALID_HANDLE]. If WSASocket() is not called, NtClose() after WSACleanup() returns STATUS_SUCCESS. The same applies to a socket created with DuplicateHandle() directly.
dlls/ws2_32/socket.c | 14 +++++++++++--- dlls/ws2_32/tests/sock.c | 3 --- 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 44214630296..246a0e2819b 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1770,9 +1770,17 @@ int WINAPI WSAStartup(WORD wVersionRequested, LPWSADATA lpWSAData) */ INT WINAPI WSACleanup(void) { - if (num_startup) { - num_startup--; - TRACE("pending cleanups: %d\n", num_startup); + TRACE("decreasing startup count from %d\n", num_startup); + if (num_startup) + { + if (!--num_startup) + { + unsigned int i; + + for (i = 0; i < socket_list_size; ++i) + CloseHandle(SOCKET2HANDLE(socket_list[i])); + memset(socket_list, 0, socket_list_size * sizeof(*socket_list)); + } return 0; } SetLastError(WSANOTINITIALISED); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index c7f88d6f3b1..7835246397d 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1221,7 +1221,6 @@ static void test_WithWSAStartup(void) ok(res == 0, "WSAStartup() failed unexpectedly: %d\n", res);
/* show that sockets are destroyed automatically after WSACleanup */ - todo_wine { SetLastError(0xdeadbeef); res = send(pairs[0].src, "TEST", 4, 0); error = WSAGetLastError(); @@ -1258,8 +1257,6 @@ static void test_WithWSAStartup(void) } }
- } - /* While wine is not fixed, close all sockets manually */ for (i = 0; i < socks; i++) {
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=78516
Your paranoid android.
=== w10pro64_ar (32 bit report) ===
ws2_32: sock.c:3832: Test failed: expected 1, got 0 sock.c:3837: Test failed: expected 10061, got 0 sock.c:3838: Test failed: fdWrite socket is not in the set
=== w1064v1809 (64 bit report) ===
ws2_32: sock.c:5921: Test failed: Expected event sequence [FD_WRITE(0)], got [] sock.c:5942: Test failed: Expected event sequence [FD_READ(0) FD_CLOSE(0)], got [FD_READ(0) FD_WRITE(0) FD_CLOSE(10053)] sock.c:5945: Test failed: Failed to empty buffer: -1 - 10054 sock.c:5946: Test failed: Expected event sequence [FD_READ(0)], got [] sock.c:5949: Test failed: Failed to empty buffer: -1 - 10054
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- server/handle.c | 21 --------------------- server/handle.h | 2 -- 2 files changed, 23 deletions(-)
diff --git a/server/handle.c b/server/handle.c index 9ae99cd0c63..10e6a52e571 100644 --- a/server/handle.c +++ b/server/handle.c @@ -486,27 +486,6 @@ obj_handle_t find_inherited_handle( struct process *process, const struct object return 0; }
-/* enumerate handles of a given type */ -/* this is needed for window stations and desktops */ -obj_handle_t enumerate_handles( struct process *process, const struct object_ops *ops, - unsigned int *index ) -{ - struct handle_table *table = process->handles; - unsigned int i; - struct handle_entry *entry; - - if (!table) return 0; - - for (i = *index, entry = &table->entries[i]; i <= table->last; i++, entry++) - { - if (!entry->ptr) continue; - if (entry->ptr->ops != ops) continue; - *index = i + 1; - return index_to_handle(i); - } - return 0; -} - /* get/set the handle reserved flags */ /* return the old flags (or -1 on error) */ static int set_handle_flags( struct process *process, obj_handle_t handle, int mask, int flags ) diff --git a/server/handle.h b/server/handle.h index f1deb79fb5f..40b3b427c2a 100644 --- a/server/handle.h +++ b/server/handle.h @@ -48,8 +48,6 @@ extern obj_handle_t open_object( struct process *process, obj_handle_t parent, u const struct object_ops *ops, const struct unicode_str *name, unsigned int attr ); extern obj_handle_t find_inherited_handle( struct process *process, const struct object_ops *ops ); -extern obj_handle_t enumerate_handles( struct process *process, const struct object_ops *ops, - unsigned int *index ); extern void close_process_handles( struct process *process ); extern struct handle_table *alloc_handle_table( struct process *process, int count ); extern struct handle_table *copy_handle_table( struct process *process, struct process *parent );
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- server/fd.c | 6 ------ server/file.h | 1 - 2 files changed, 7 deletions(-)
diff --git a/server/fd.c b/server/fd.c index 7ea8ac273e5..a88ba39ef01 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2057,12 +2057,6 @@ void set_fd_signaled( struct fd *fd, int signaled ) if (signaled) wake_up( fd->user, 0 ); }
-/* check if fd is signaled */ -int is_fd_signaled( struct fd *fd ) -{ - return fd->signaled; -} - /* handler for close_handle that refuses to close fd-associated handles in other processes */ int fd_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) { diff --git a/server/file.h b/server/file.h index e8ace7f49e4..b02c9fe6037 100644 --- a/server/file.h +++ b/server/file.h @@ -99,7 +99,6 @@ extern obj_handle_t lock_fd( struct fd *fd, file_pos_t offset, file_pos_t count, extern void unlock_fd( struct fd *fd, file_pos_t offset, file_pos_t count ); extern void allow_fd_caching( struct fd *fd ); extern void set_fd_signaled( struct fd *fd, int signaled ); -extern int is_fd_signaled( struct fd *fd ); extern char *dup_fd_name( struct fd *root, const char *name );
extern int default_fd_signaled( struct object *obj, struct wait_queue_entry *entry );
-
Marvin -
Zebediah Figura