Skip to content

SocketHandle::new leaks WSA event when WSAEventSelect fails #2

Description

@Parikalp-Bhardwaj

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions