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

websocket: synchronization between weboscket manager callback and incoming message callback #126

Closed
stefvanos opened this issue Jul 4, 2019 · 4 comments

Comments

@stefvanos
Copy link

Hi,

I need to be able to have access to the same per-websocket session data in both the manager (asynchronous writes to websocket) and incoming message callback (reads). For multiple sessions with their own private data, there is no way at the moment to share private data between websocket manager and websocket incoming message callback.

I can think of 2 ways of doing this, but both are not an option at the moment:

  1. Attach private data to "struct _websocket_manager"
    This way I could create a pipe in private to notify manager callback that data has arrived from incoming message callback. Currently there is no field in a "struct _websocket_manager" to do this.

I could allocate a private data struct per session before calling "ulfius_set_websocket_response" and provide the same pointer for user data for all 3 callbacks, but if the handshake fails there is no way to know and free the private data (ulfius_set_websocket_response returns U_OK but callbacks are never called)

  1. Use poll directly from manager callback
    Skip incoming message callback all-together and do everything from manager callback. I can't find a way to wait for incoming data with poll/select in the current API.

Maybe I'm using ulfius wrong, but is there a way to wait for incoming data (using e.g. poll) in the websocket manager callback?

Best regards, and thanks in advance!

@babelouest
Copy link
Owner

Hello,

If you want to share data between websocket_manager_callback and websocket_incoming_message_callback, you can use the same *_user_data when you call ulfius_set_websocket_response, like in the websocket_server.c example.

This way you can share a session between those callback functions.
In Taliesin for example, I use a pointer of a structure with the following definition:

struct _ws_stream {
  struct config_elements * config;
  char                   * username;
  int                      is_admin;
  int                      is_authenticated;
  time_t                   expiration;
  struct _t_webradio     * webradio;
  struct _t_jukebox      * jukebox;
  int                      status;
};

More specifically, if you want, in the websocket_manager_callback, to be notified of incoming messages, I see at least 2 solutions:

  • you make an infinite loop and check for new messages in websocket_manager->message_list_incoming => simple but a little bit ugly
  • you use a pthread condition, in websocket_incoming_message_callback you broadcast a message and in websocket_manager_callback, you wait for this signal => a little more lines of code but way more secure and clean

@stefvanos
Copy link
Author

Hi,

I Think (correct me if I miss something) that that code leaks the malloced handle if you do a normal http request (not websocket) on that endpoint

  1. the "callback_taliesin_stream_manage_ws" is called, allocates the handle.
  2. ulfius_set_websocket_response is called, returns U_OK, ws_stream is not freed because success
  3. websock handshake begins
  4. websock handshake fails
  5. none of the 3 callbacks is ever called to free ws_stream

I now allocate the private data in the manager thread and connect it to the "websocket_manager_user_data" pointer from there. Taking into account a small race condition there, but it's a workable solution for now.

If i am mistaking about the above leak, please excuse me and this issue can be closed

BR,
Stef

@babelouest
Copy link
Owner

Hello @stefvanos ,

Concerning the leak, I'm afraid you're right.

If you allocate memory and pass it to the ulfius_set_websocket_response as *_user_data, but for some reason the websocket handshake isn't valid, then websocket_onclose_callback isn't called, so the leak may remain until the end of the program...
I've tested it with the websocket_example and I confirm. Thanks for noticing!

By design, ulfius_set_websocket_response returns U_OK if the parameters passed to this function are valid, not if the http request is a valid websocket request, and the handshake validation starts after you return from the http callback function.

I'll make a patch to execute the callback function websocket_onclose_callback in all cases and update the documentation accordingly.

@babelouest babelouest added the bug label Jul 5, 2019
babelouest added a commit that referenced this issue Jul 6, 2019
… fixed, otherwise one shouldn't call that a fix, or not a full one, more someting like a half-fix or a semi-fix, or a not-so-fix, pick your best
@babelouest
Copy link
Owner

@stefvanos ,

The leak has been fixed in the commit 47b2b4c , let me know if you have other issues, thanks!

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

No branches or pull requests

2 participants