Theldus / wsServer

wsServer - a tiny WebSocket server library written in C
https://theldus.github.io/wsServer
GNU General Public License v3.0
422 stars 80 forks source link

mutiple sockets not working. #85

Closed ihmpartners closed 7 months ago

ihmpartners commented 9 months ago

Description

I have a small utility serving up 3 sockets on 6060, 6061 and 6062.

I have previously compiled 3 independent files, each serving up one of the 3 sockets and this appears to work perfectly but if I combine the code into a single file and compile it, all of the responses are served up by a single message receiver.

Your environment

Both on Linux 22 and stm32embedded

my C code

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>

#include "../include/ws.h"

#include "data.h"
#include "graph.h"
#include "alerts.h"

#define MAX_PORTS   9

/**
 * @dir examples/
 * @brief wsServer examples folder
 */

/*
 * @dir examples/echo
 * @brief Echo example directory.
 * @file echo.c
 * @brief Simple echo example.
 */

/**
 * @brief Called when a client connects to the server.
 *
 * @param client Client connection. The @p client parameter is used
 * in order to send messages and retrieve informations about the
 * client.
 */
void onDataOpen(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
#ifndef DISABLE_VERBOSE
    printf("Data Connection opened, addr: %s\n", cli);
#endif
}

void onGraphOpen(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
#ifndef DISABLE_VERBOSE
    printf("Graph Connection opened, addr: %s\n", cli);
#endif
}

void onAlertsOpen(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
  #ifndef DISABLE_VERBOSE
    printf("Alerts Connection opened, addr: %s\n", cli);
  #endif
}

void onDataClose(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
#ifndef DISABLE_VERBOSE
    printf("Data Connection closed, addr: %s\n", cli);
#endif
}

void onGraphClose(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
#ifndef DISABLE_VERBOSE
    printf("Graph Connection closed, addr: %s\n", cli);
#endif
}

void onAlertsClose(ws_cli_conn_t *client)
{
    char *cli;
    cli = ws_getaddress(client);
#ifndef DISABLE_VERBOSE
    printf("Alerts Connection closed, addr: %s\n", cli);
#endif
}

void onDataMessage(ws_cli_conn_t *client,   const unsigned char *msg, uint64_t size, int type) {
    char *cli;
    cli = ws_getaddress(client);

  #ifndef DISABLE_VERBOSE
    printf("Received a Data message: %s (size: %" PRId64 ", type: %d), from: %s\n", msg, size, type, cli);
  #endif
     ws_sendframe(NULL, &dvalues[0], strlen(dvalues), type);
     }

void onGraphMessage(ws_cli_conn_t *client,  const unsigned char *msg, uint64_t size, int type) {
     char *cli;
     cli = ws_getaddress(client);    

     #ifndef DISABLE_VERBOSE
     printf("Received a Graph message: %s (size: %" PRId64 ", type: %d), from: %s\n", msg, size, type, cli);
     #endif

     ws_sendframe(NULL, &gvalues[0], strlen(gvalues), type);
     }

void onAlertMessage(ws_cli_conn_t *client,  const unsigned char *msg, uint64_t size, int type) {
     char *cli;
     cli = ws_getaddress(client);
     #ifndef DISABLE_VERBOSE
     printf("Received an Alert message: %s (size: %" PRId64 ", type: %d), from: %s\n", msg, size, type, cli);
   #endif

     ws_sendframe(NULL, &avalues[0], strlen(avalues), type);
     }

/**
 * @brief Main routine.
 *
 * @note After invoking @ref ws_socket, this routine never returns,
 * unless if invoked from a different thread.
 */
int main(void)
{

  ws_socket(&(struct ws_server){
    .host                    = "0.0.0.0",
    .port                = 6060,
    .thread_loop   = 1,
    .timeout_ms    = 10000,
        .evs.onopen    = &onDataOpen,
        .evs.onclose   = &onDataClose,
        .evs.onmessage = &onDataMessage
        });  

  ws_socket(&(struct ws_server){
    .host                    = "0.0.0.0",
    .port                = 6061,
    .thread_loop   = 1,
    .timeout_ms    = 10000,
        .evs.onopen    = &onGraphOpen,
        .evs.onclose   = &onGraphClose,
        .evs.onmessage = &onGraphMessage
        });    

  ws_socket(&(struct ws_server){
    .host                    = "0.0.0.0",
    .port            = 6062,
    .thread_loop     = 1,
    .timeout_ms      = 10000,
        .evs.onopen    = &onAlertsOpen,
        .evs.onclose   = &onAlertsClose,
        .evs.onmessage = &onAlertMessage
        });     

    while(1){
         int i = 0;
         i++;
         if (i>10000) {
             i = 0;
         }
    };

    /*
     * If you want to execute code past ws_socket, invoke it as follows:
     *   ws_socket(&evs, 6060, 1, 1000);
     */

    return (0);
}

Three sockets are opened on 6060,6061 and 6062,

All callbacks are serviced by the AlertMessage callbacks. ( OnAlertsOpen, OnAlertsClose and onAlertMessage )

ihmpartners commented 9 months ago

a bit of a development ......

If the order of the ws_socket calls to create the sockets is changed then the last one in the list becomes the set that services events.

int main(void)
{
    struct ws_server data_srv;
    data_srv.host = "0.0.0.0";
    data_srv.port                = 6060;
    data_srv.thread_loop   = 1;
  data_srv.timeout_ms    = 10000,
    data_srv.evs.onopen    = &onDataOpen;
    data_srv.evs.onclose   = &onDataClose;
    data_srv.evs.onmessage = &onDataMessage;

  ws_socket(&(struct ws_server){
    .host                    = "0.0.0.0",
    .port                = 6061,
    .thread_loop   = 1,
    .timeout_ms    = 10000,
        .evs.onopen    = &onGraphOpen,
        .evs.onclose   = &onGraphClose,
        .evs.onmessage = &onGraphMessage
        });    

  ws_socket(&(struct ws_server){
    .host                    = "0.0.0.0",
    .port            = 6062,
    .thread_loop     = 1,
    .timeout_ms      = 10000,
        .evs.onopen    = &onAlertsOpen,
        .evs.onclose   = &onAlertsClose,
        .evs.onmessage = &onAlertMessage
        });     

  ws_socket(&data_srv);     

and all events are serviced by the Data events.

ihmpartners commented 9 months ago

I think I need some threads .........

Theldus commented 9 months ago

Hi @ihmpartners, This is indeed a bug that I had never noticed, thank you very much for reporting it.

Commit 2169643d7d47869098f58d372bd0fcf8ec8d1eaf should fix this. Please let me know if this works for you.


Edit: Tip: You don't need to do an infinite loop to keep the main thread of the program blocking, you can just set .thread_loop = 0 in the last call to ws_socket(), as in:

...

ws_socket(&(struct ws_server){
    .host = "0.0.0.0",
    .port = 6060,
    .thread_loop   = 1,
    .timeout_ms    = 10000,
    .evs.onopen    = &onDataOpen,
    .evs.onclose   = &onDataClose,
    .evs.onmessage = &onDataMessage
});
ws_socket(&(struct ws_server){
    .host = "0.0.0.0",
    .port = 6061,
    .thread_loop = 1,
    .timeout_ms  = 10000,
    .evs.onopen    = &onGraphOpen,
    .evs.onclose   = &onGraphClose,
    .evs.onmessage = &onGraphMessage
});
ws_socket(&(struct ws_server){
    .host = "0.0.0.0",
    .port = 6062,
    .thread_loop = 0, /* <-------------------- here. */
    .timeout_ms  = 10000,
    .evs.onopen    = &onAlertsOpen,
    .evs.onclose   = &onAlertsClose,
    .evs.onmessage = &onAlertMessage
});

so you do not waste CPU cycles πŸ˜‰.

ihmpartners commented 9 months ago

Hi.

Yes I am adding some more code after the socket instantiations so for the moment ...... the loop code.

I am doing some testing but currently I appear to still have a problem.

1 - Open my 3 servers 6060,6061 and 6062 and 2 - Open the clients from a single web page. once each connection is established a simple text message is passed to the each socket which responds with a message

The following shows how the server responds. ( Chrome Web dignostics )

Subscribe to Data 6060
ws_sockets.js:23 Data Socket RECV: {"test": "D channel 6060"}
setInstGraph.js:54 Subscribe to Graph 6061
ws_sockets.js:41 Graph Socket RECV: {"test": "D channel 6060"}
ws_sockets.js:41 Graph Socket RECV: {"test": "G channel 6061"}
setInstGraph.js:60 Subscribe to Alerts 6062
ws_sockets.js:41 Graph Socket RECV: {"test": "G channel 6061"}
(2)ws_sockets.js:59 Alert Socket RECV: {"test": "G channel 6061"}
(3)ws_sockets.js:23 Data Socket RECV: {"test": "G channel 6061"}
ws_sockets.js:41 Graph Socket RECV: {"test": "G channel 6061"}

subscribe to .... is sent from the browser ws_sockets.js nn is each service routine showing the data it receives. so in this case. ( so far so good ) 6060 opened, server on 6060 responds to 6060. (2) 6061 opened, server on 6060 responds to 6060 and then server on 6061 responds to 6061 ( why is 6060 in here and response received twice ) (3) 6062 opened, server on 6061 responds to 6061 6062 and then to 6060 ( received 3 times - some of this tranche may be from the first or second subscribe as opening 2 sockets also results in mixed messages ) a last message is sent to the 6061 socket from the server service routine on 6061

as you can see it looks as if as stack of calls are being repeated. I'll have a look at the source but I am snowed under with other work atm! )

it looks as if a stack of calls are being itterated through from the bottom rather than from some index point ..... you get my meaning ....

Ahh .... to add to the problem. separate browser instances do not open unique sockets, refreshing browser 1 results in identical data being transmitted to browser 2. ( this doesn't happen with the last build if multiple instances of the process are started, each sericing a single port )

Theldus commented 9 months ago

Hi @ihmpartners, This is weird, I can't reproduce your issue here.... connecting to the example html (examples/echo/echo.html) on three different ports, messages are only sent and received to that port, messages are not mixed .

Could you provide me a minimal reproducible example that shows this problem? (html + .c)

ihmpartners commented 9 months ago

will do.

attached a html file and my server C code. ( and a compiled version for Lunux Ubuntu 22.04 )

and a 'triple' version of echo HTML which wil connect to 6060 6061 and 6062 ( defaults for the server ports )

I am currently running this on lighttpd. but I will run up an Apache instance just to be sure.

Video of my issue with the above code/s https://www.youtube.com/watch?v=ci8yS4nUZ10

regards Ian

On Wed, 6 Dec 2023 at 21:46, Davidson Francis @.***> wrote:

Hi @ihmpartners https://github.com/ihmpartners, This is weird, I can't reproduce your issue here.... connecting to the example html (examples/echo/echo.html) on three different ports, messages are only sent and received to that port, messages are not mixed .

Could you provide me a minimal reproducible example https://stackoverflow.com/help/minimal-reproducible-example that shows this problem? (html + .c)

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1843741628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSV56SIIJQ2KN3HJFBDYIDRUNAVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTG42DCNRSHA . You are receiving this because you were mentioned.Message ID: @.***>

ihmpartners commented 9 months ago

Apache2 has the same problem so that counts that out.

On Fri, 8 Dec 2023 at 08:38, Ian Molesworth @.***> wrote:

will do.

attached a html file and my server C code. ( and a compiled version for Lunux Ubuntu 22.04 )

and a 'triple' version of echo HTML which wil connect to 6060 6061 and 6062 ( defaults for the server ports )

I am currently running this on lighttpd. but I will run up an Apache instance just to be sure.

Video of my issue with the above code/s https://www.youtube.com/watch?v=ci8yS4nUZ10

regards Ian

On Wed, 6 Dec 2023 at 21:46, Davidson Francis @.***> wrote:

Hi @ihmpartners https://github.com/ihmpartners, This is weird, I can't reproduce your issue here.... connecting to the example html (examples/echo/echo.html) on three different ports, messages are only sent and received to that port, messages are not mixed .

Could you provide me a minimal reproducible example https://stackoverflow.com/help/minimal-reproducible-example that shows this problem? (html + .c)

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1843741628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSV56SIIJQ2KN3HJFBDYIDRUNAVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTG42DCNRSHA . You are receiving this because you were mentioned.Message ID: @.***>

Theldus commented 8 months ago

Hi @ihmpartners, (Sorry for the delay; I've been quite busy in the last few days.)

Your YouTube video is set to private, and your message doesn't have any attachments. If you could look into this, I would appreciate it.

Theldus commented 8 months ago

@ihmpartners Issue #86 may be related to yours and I've pushed a fix for it. Could you check if this fix works for you too?

ihmpartners commented 8 months ago

Ah. I wondered about that. Will resend it all.

On Thu, 14 Dec 2023, 03:39 Davidson Francis, @.***> wrote:

@ihmpartners https://github.com/ihmpartners Issue #86 https://github.com/Theldus/wsServer/issues/86 may be related to yours and I've pushed a fix for it. Could you check if this fix works for you too?

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1855076320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSQLCZGNE6JP35HXQP3YJJYHJAVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJVGA3TMMZSGA . You are receiving this because you were mentioned.Message ID: @.***>

Theldus commented 8 months ago

@ihmpartners I've now noticed that your video is publicly available; thank you for the detailed demonstration. I finally understand what's going on.

In commit cb2c3f8 (PR #45), a code refactoring was performed, and the signature of the ws_sendframe() function was simplified from:

int ws_sendframe(int fd, const char *msg, uint64_t size, bool broadcast, int type);

to:

int ws_sendframe(ws_cli_conn_t *client, const char *msg, uint64_t size, int type);

Additionally, the broadcast functionality is now signaled when 'client' is NULL.

The issue is that, with 'client' being NULL, it becomes impossible to determine which port to send the broadcast message to. As a result, it is sent to all connected clients, regardless of their associated ports. This is why you were receiving mixed messages.

Unfortunately, this is not a bug, but at the same time, it is unexpected behavior for the user. Perhaps I should reintroduce the 'broadcast' parameter in the ws_sendframe()/ws_sendframe_txt()/ws_sendframe_bin() routines. What do you think? Any suggestions?

The only issue is that it might become a bit confusing to use:

if: client != true && broadcast != true -> ??? (error?)
if: client != true && broadcast == true -> broadcast to all clients
if: client == true && broadcast != true -> message to the specified client
if: client == true && broadcast == true -> broadcast to the client's port

edit: The 'broadcast' parameter could be an integer instead of a boolean, with the following values:

NO_BROADCAST   (0): No broadcast.
ALL_BROADCAST  (1): Broadcast to all clients (regardless of the port).
PORT_BROADCAST (2): Broadcast to all clients on the same port as 'client'.

In this way, the 'client' parameter becomes mandatory rather than optional, but I believe it simplifies the API. What are your thoughts on this?

edit2: The proposal above requires users to have prior knowledge of a client associated with the port they want to broadcast to, which may not be ideal...

Perhaps, then, introducing separate methods for broadcasting?

ws_sendframe()
ws_sendframe_txt()
ws_sendframe_bin()

ws_sendframe_port(6060, ...)
ws_sendframe_txt_port(6060, ...)
ws_sendframe_bin_port(6060, ...)

The first three methods maintain their current behavior: if NULL, broadcast to all. The last three methods allow the selection of a port without specifying a particular client.

Or some variation of this...

edit3: Or maybe a general broadcast (ignoring ports) doesn't make sense...

ihmpartners commented 8 months ago

A broadcast function would be understandable.

My requirement is that each client receives only data intended for it.

I could deal with having everything sent to all but its going to be a real pain. A bit like having a web server that delivers every page to every connected client.

On Thu, 14 Dec 2023, 18:41 Davidson Francis, @.***> wrote:

@ihmpartners https://github.com/ihmpartners I've now noticed that your video is publicly available; thank you for the detailed demonstration. I finally understand what's going on.

In commit cb2c3f8 https://github.com/Theldus/wsServer/commit/cb2c3f80e44ef0ded8bbc728b73d9f803c7bd3ba (PR #45 https://github.com/Theldus/wsServer/pull/45), a code refactoring was performed, and the signature of the ws_sendframe() function was simplified from:

int ws_sendframe(int fd, const char *msg, uint64_t size, bool broadcast, int type);

to:

int ws_sendframe(ws_cli_conn_t client, const char msg, uint64_t size, int type);

Additionally, the broadcast functionality is now signaled when 'client' is NULL.

The issue is that, with 'client' being NULL, it becomes impossible to determine which port to send the broadcast message to. As a result, it is sent to all connected clients, regardless of their associated ports. This is why you were receiving mixed messages.

Unfortunately, this is not a bug, but at the same time, it is unexpected behavior for the user. Perhaps I should reintroduce the 'broadcast' parameter in the ws_sendframe()/ws_sendframe_txt()/ws_sendframe_bin() routines. What do you think? Any suggestions?

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1856392453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSRTV5YAYTU2ARXP46TYJNB5LAVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJWGM4TENBVGM . You are receiving this because you were mentioned.Message ID: @.***>

Theldus commented 8 months ago

Hi @ihmpartners, Commit fbc1547 changes the semantics of the ws_sendframe*() functions and introduces three new functions: ws_sendframe_bcast(), ws_sendframe_txt_bcast(), and ws_sendframe_bin_bcast().

These new functions are intended for broadcasting and take the port as a parameter instead of the client, as follows:

ws_sendframe_bcast(6062, msg, size, type);
ws_sendframe_txt_bcast(6062, msg);
ws_sendframe_bin_bcast(6062, msg, size);

The old functions still exist, but they no longer send broadcast messages, and passing a NULL client to them is now considered an error!

Please let me know if these changes works for you, and thank you very much for your persistence in helping me resolve this issue. I greatly appreciate it.

ihmpartners commented 7 months ago

Sorry for the delayed response but busy carving stuff out for other parts of the project.

Working great now. individual clients, even multiple ones on the same machine now correctly route messages only to that client!

Regards Ian M

On Fri, 15 Dec 2023 at 06:14, Davidson Francis @.***> wrote:

Hi @ihmpartners https://github.com/ihmpartners, Commit fbc1547 https://github.com/Theldus/wsServer/commit/fbc1547a1a361594c72dd0cf897c937c905032f6 changes the semantics of the ws_sendframe*() functions and introduces three new functions: ws_sendframe_bcast(), ws_sendframe_txt_bcast(), and ws_sendframe_bin_bcast().

These new functions are intended for broadcasting and take the port as a parameter instead of the client, as follows:

ws_sendframe_bcast(6062, msg, size, type);ws_sendframe_txt_bcast(6062, msg);ws_sendframe_bin_bcast(6062, msg, size);

The old functions still exist, but they no longer send broadcast messages, and passing a NULL client to them is now considered an error!

Please let me know if these changes works for you, and thank you very much for your persistence in helping me resolve this issue. I greatly appreciate it.

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1857336515, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSS6BGVS4Q5OYAR4XW3YJPTC7AVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJXGMZTMNJRGU . You are receiving this because you were mentioned.Message ID: @.***>

Theldus commented 7 months ago

Hi @ihmpartners, Glad to hear it works well for you now, I'll close the issue, but feel free to open another one for any issues you have.

ihmpartners commented 7 months ago

Thanks.

On Fri, 2 Feb 2024, 17:27 Davidson Francis, @.***> wrote:

Hi @ihmpartners https://github.com/ihmpartners, Glad to hear it works well for you now, I'll close the issue, but feel free to open another one for any issues you have.

β€” Reply to this email directly, view it on GitHub https://github.com/Theldus/wsServer/issues/85#issuecomment-1924353729, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARIWSTHLYNYSOEGG6GAZKDYRUOX3AVCNFSM6AAAAABAHWLJQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRUGM2TGNZSHE . You are receiving this because you were mentioned.Message ID: @.***>