Kitura / Kitura-WebSocket

WebSocket support for Kitura
Apache License 2.0
68 stars 30 forks source link

Request headers not available in connected callback #12

Closed bridger closed 7 years ago

bridger commented 7 years ago

In my WebSocketService I'd like to inspect some headers on a new websocket connection. The headers are often incorrectly shown as empty when read from the connected(connection: WebSocketConnection) callback.

I've done some debugging and found out the root cause. The HTTPServerRequest object doesn't strongly retain the headers. Instead, it has a weak reference to a httpParser which contains the headers. The connected callback is called asynchronously here, by which time the httpParser is often released.

My guess is that the WebSocketConnection should retain the httpParser to make sure the data stays alive for the lifetime of the connection. I'm not sure if this is safe to do, though.

In production this bug doesn't happen 100% of the time. It depends on the timing of the two threads. I think this should repro pretty consistently:

func connected(connection: WebSocketConnection) {
   print("Headers usually present here: \(Array(connection.request.headers))")
   sleep(3) // Give time for other thread to release the httpParser
   print("Headers usually empty here: \(Array(connection.request.headers))")
}
bridger commented 7 years ago

Meta-question: Is this the right place to talk about bugs? Or should all bugs go to the main Kitura repo? This bug in particular might need fixes in Kitura-Net, for example.

shmuelk commented 7 years ago

I have a fix for this pushed in the branch issue_12. It also needs the latest Kitura-net (1.7.3).

This should get merged tomorrow morning.

bridger commented 7 years ago

Thank you! It works great now. I really appreciate the great work you've done with this framework.