Associate userdata to a connection

[Thalhammer]

I really like this project, however, I found one pretty big problem for me. I tend to have data associated with a connection (like authentication, jsonrpc states and similar). This would normally involve a mapping between seasocks::WebSocket* and my data, which makes it more complex and hurts multithreading.

This pull request adds a userdata field to the connection class (currently a shared_ptr, to allow for RAII, but feel free to change it to a plain void* if you like to). This allows for simple and fast access to all required data.

[hoytech]

I too have always had to use a Connection struct and a mapping, basically something like this:

std::unordered_map<seasocks::WebSocket*, Connection> websocket_to_connection;

So I have no objection to this change, although is a shared pointer really necessary here?

Any thoughts @mattgodbolt @offa ?

[mattgodbolt]

@hoytech Right: that's another way of doing it (and often one wants to be able to iterate over "all connections" anyway). Thoughts @Thalhammer ?

[Thalhammer]

@hoytech Yeah that's how its usually done, but it presents a potential problem if you want to do multithreading as you need to lock this map while you do anything with it. This effectively limits your concurrency to less than 1.

I don't think a single shared_ptr hurts anyone and makes this stuff way easier. However, that's only my personal opinion so if you don't think it's appropriate here feel free to reject the PR.

[hoytech]

Can you please elaborate on how you intend to do multi-threading? I've always been careful to only interact with the Server object from the seasocks thread, as it describes in the documentation.

My technique is for other threads to push outbound messages onto a lock-protected queue and then call the server->execute() method to wake-up the seasocks thread which reads from the queue. Each message has a "connectionId" which is an always-increasing counter I give to every connection. When it pops a message, the seasocks thread checks an unordered_map conn_id_to_connection to see if that connection still exists, and if so sends the message to it.

I'm trying to understand how this would work with a Connection struct protected by a shared_ptr. Thanks!

[Thalhammer]

@hoytech Never mind, I totally forgot that seasocks does not support multithreading at all.

@mattgodbolt I guess it comes down to personal preference and as seasocks does not support multithreading at all (and does not intend to do), I am closing this, as it does not provide a benefit for most people.

[hoytech]

Not so fast! I like the concept of a user data pointer stored in the WebSocket struct. It would save a lookup in a map and simplify the application code.

My preference would be either a void* and leave it up to the app (this is what uWebSockets does BTW) or maybe a unique_ptr, if we can figure out the templating or inheritance or whatever.

[mattgodbolt]

@hoytech some thoughts:

• void* has potential lifetime issues, essentially punting and making it the users' problem.
• unique_ptr requires that seasocks "owns" the data (which is probably ok), but means that the data needs to be of a known type (probably a known base type with a virtual destructor).
• shared_ptr has the benefit of laundering the destructor for free

Happy to consider any of these though: I can see if being useful.

[Thalhammer]

I'd like to throw in a fourth option: Store both void* and a deleter function (So basically a unique_ptr but without the need to know the type).

[hoytech]

I think any of them are fine, but personally would go for void*. There is a symmetry that appeals to me: Construct and set your user data in onConnect, and retrieve and delete it in onDisconnect. The library doesn't concern itself about how you construct/delete anything.

I don't see the lifecycle issues being a big deal personally. As it stands now, if you keep any connection-specific state you have to remember to clean it up in onDisconnect anyway, and thankfully this is the only place you need to do so.