On Thu Apr 17 19:59:06 2025 +0000, Elizabeth Figura wrote:
recv() doesn't have it, and the whole idea of local socket cache seems a little bit fragile, especially wrt duplicated handles.It's certainly fragile, but it's how native behaves. It's not hard to demonstrate that native has a process-local list that it refers to in some cases, and that this indeed breaks when you use DuplicateHandle() directly. In fact, as far as Microsoft is concerned, you're broadly not supposed to do that; you're supposed to use WSADuplicateSocketW() and WSASocket() on the other end. I believe that this is also what SO_UPDATE_ACCEPT_CONTEXT and SO_UPDATE_CONNECT_CONTEXT are for in some sense—I don't think everything works without them, although I'm less sure about that one. I don't think this patch should be committed as long as it breaks todos. It's possible that only some calls in the send() family should check the socket list, and we should move the socket_list_find() call to those specific callers. Or possibly this one is only here because of an error condition ordering problem, which may be trickier to address.
The patch doesn't really break the todos, although it may look like it. Rather the affected tests are broken because it seems that they will fail even when called with a valid socket handle, what will break them is the next condition
if (!overlapped && !ret_size)
{
SetLastError( WSAEFAULT );
return -1;
}
So, it seems that it's better to set wrong last error value than completely fail send().