Karlson2k / libmicrohttpd

GNU libmicrohttpd repository unofficial mirror on GitHub
https://www.gnu.org/software/libmicrohttpd/
Other
101 stars 29 forks source link

thread-safety of "MHD_websocket_encode_*" functions #28

Closed jvo203 closed 7 months ago

jvo203 commented 7 months ago

It's more of a question than an issue and I realise the WebSocket functionality is still experimental. Whilst adapting your WebSocket Chat Server example so suit my project there has been this nagging issue: do all the WebSocket stream encode/decode functions like MHD_websocket_encode_pong(ws, ...), MHD_websocket_encode_text(ws, ...) and MHD_websocket_encode_binary(ws, ...) need a mutex protection in a fully multi-threaded environment?

Given a single unique WebSocket stream session->ws and a number of producer POSIX threads creating WebSocket messages on the server, does one need to do this

    // lock the encode_mtx
    pthread_mutex_lock(&session->encode_mtx);

    int er = MHD_websocket_encode_binary(session->ws,
                                         data,
                                         data_len,
                                         MHD_WEBSOCKET_FRAGMENTATION_NONE,
                                         &frame_data,
                                         &frame_len);

    // unlock the encode_mtx
    pthread_mutex_unlock(&session->encode_mtx);

prior to calling send_all() or is the encode_mtx completely unnecessary? Each producer thread operates on its own frame_data, frame_len variables. In addition, the send_all() internally locks a separate send_mtx to protect the TCP WebSocket writes:

/**
 * Sends all data of the given buffer via the TCP/IP socket
 *
 * @param fd  The TCP/IP socket which is used for sending
 * @param buf The buffer with the data to send
 * @param len The length in bytes of the data in the buffer
 */
static void send_all(websocket_session *session, const char *buf, size_t len)
{
    if (pthread_mutex_lock(&session->send_mutex) == 0)
    {
        ssize_t ret;
        size_t off;

        for (off = 0; off < len; off += ret)
        {
            ret = send(session->fd, &buf[off], (int)(len - off), 0);

            if (0 > ret)
            {
                if (EAGAIN == errno)
                {
                    ret = 0;
                    continue;
                }
                break;
            }

            if (0 == ret)
                break;
        }

        pthread_mutex_unlock(&session->send_mutex);
    }
    else
        printf("[C] <send_all(%zu bytes)> failed, cannot lock the WebSocket send_mutex!\n", len);
}

I realise there is also another - albeit much more low-level - example that can be followed: websocket_threaded_example.c. It does not rely on a unique WebSocket stream session->ws.

libmicrohttpd_ws is still experimental but it's great! It certainly beats the mongoose networking library, as per the attached PDF slide (ignore the 1st page, written in Japanese). libmicrohttpd.pdf

jvo203 commented 7 months ago

Closing this issue as I have implemented a low-level fix: creating text/binary/pong WebSocket messages directly in my application code. This completely avoids having to call libmicrohttpd MHD_websocket_encode_* functions. All libmicrohttpd does is simply to upgrade the HTTP connection to WebSocket and decode the incoming WebSocket stream. All the responses are handled at a lower level, which results in improved performance (avoids the extra memory allocation step inside the encode_* functions).