Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HCGetWebSocketConnectResult populates result with different handle #697

Closed
redbaron opened this issue Jun 4, 2022 · 3 comments
Closed

Comments

@redbaron
Copy link
Contributor

redbaron commented Jun 4, 2022

HCGetWebSocketConnectResult seems to populate result with a different websocket handle than one used to call HCWebSocketConnectAsync. When multiple simultaneous connections are used I wanted to use WebSocketCompletionResult::websocket to disambiguate between them, but it seems not possible.

auto* asyncBlock = new XAsyncBlock{};
asyncBlock->context = ws;
asyncBlock->queue = q;
asyncBlock->callback = [](XAsyncBlock *ab) {
    const std::unique_ptr<XAsyncBlock> asyncBlock(ab);
    auto ws = static_cast<HCWebsocketHandle>(asyncBlock->context);

    WebSocketCompletionResult result{};
    HRESULT hr = HCGetWebSocketConnectResult(asyncBlock.get(), &result);
    if (SUCCEEDED(hr)) {
       assert(ws == result.websocket);  // <<< FAILS
    }
};

HCWebSocketConnectAsync(URL, "",  ws, asyncBlock);
@jplafonta
Copy link
Collaborator

You are correct, this was somewhat intentional. After calling HCWebSocketConnectAsync, the internal WebSocket object will be kept alive by libHttpClient until it is disconnected - even if the client closes their handle. The handle populated in the result is guaranteed to be a valid handle to the WebSocket, while the client's handle isn't.

In theory, you should be able to disambiguate between which WebSocket is connection is completing based on the XAsync context, but I also recognize that using the returned handle would be easier/more intuitive. I will change this to pass back the client's handle as part of the connect result, but the library still makes no guarantees that the handle is valid as it's still owned by the client.

@redbaron
Copy link
Contributor Author

redbaron commented Jun 8, 2022

In actual app context is already taken by this :) Your plan to pass handle used in connect call is very welcome. It will also make on connect callback more alike rest of callbacks on the websocket.

@jplafonta
Copy link
Collaborator

Client's handle will now be returned in the async result after this change: #699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants