Hi,
While looking through the Windows socket handling code, I noticed a possible resource-cleanup issue in:
src/connection/windows/helpers/socket_handle.rs
SocketHandle::new() first creates a WSAEVENT using WSACreateEvent() and then registers it with the socket using WSAEventSelect().
Currently, if WSAEventSelect() fails, the function returns an error directly:
let event: WSAEVENT = match unsafe { WSACreateEvent() } {
Ok(event) => event,
Err(error) => {
log::error!("Cannot create socket event handle: WSACreateEvent failed");
return Err(io::Error::new(ErrorKind::Other, error));
}
};
let handle: HANDLE = HANDLE(event.0 as *mut _);
let select_result =
unsafe { WSAEventSelect(raw_socket, Some(event), (FD_READ | FD_WRITE) as i32) };
if select_result == 0 {
Ok(SocketHandle {
raw_socket,
handle,
event,
})
} else {
Err(io::Error::new(
ErrorKind::Other,
"Cannot create socket event handle: WSAEventSelect failed",
))
}
The WSAEVENT is normally closed in Drop for SocketHandle, but on the WSAEventSelect() failure path a SocketHandle is never constructed, so it looks like the created event is not released.
Would it make sense to close the event with WSACloseEvent() before returning the error?
Alternatively, would you prefer using a small RAII guard so the event is automatically cleaned up until ownership is transferred to SocketHandle?
If this is considered worth fixing, I'd be happy to prepare a small PR and add a Windows-specific regression test if there is a preferred way to exercise this failure path.
Hi,
While looking through the Windows socket handling code, I noticed a possible resource-cleanup issue in:
src/connection/windows/helpers/socket_handle.rsSocketHandle::new()first creates aWSAEVENTusingWSACreateEvent()and then registers it with the socket usingWSAEventSelect().Currently, if
WSAEventSelect()fails, the function returns an error directly:The
WSAEVENTis normally closed inDrop for SocketHandle, but on theWSAEventSelect()failure path aSocketHandleis never constructed, so it looks like the created event is not released.Would it make sense to close the event with
WSACloseEvent()before returning the error?Alternatively, would you prefer using a small RAII guard so the event is automatically cleaned up until ownership is transferred to
SocketHandle?If this is considered worth fixing, I'd be happy to prepare a small PR and add a Windows-specific regression test if there is a preferred way to exercise this failure path.