"Junker, Gregory" <gregory.junker@intel.com>
Date:
12/04/2014 12:30 AM
To:
"libmicrohttpd@gnu.org" <libmicrohttpd@gnu.org>
Hi all
I need a tiny addition made to connection.c.
In keepalive_possible(), can we change the line that says
if (0 == strcasecmp (end, "close"))
to
if (0 == strcasecmp (end, "close") || 0 == strcasecmp (end, "upgrade"))
?
This would make it possible to use libmicrohttpd in a WebSocket (RFC6455) environment. Currently, the way that the WebSocket protocol works, it sends "Connection: Upgrade" in the headers with the initial handshake, which causes this check to fail and (ultimately) insert "Connection: Keep-Alive" in the response headers (which cause any compliant WebSocket client to fail the handshake, since it needs "Connection: Upgrade" in the response headers, and there is no way to remove this keepalive header from the response from outside of MHD, as it is added automatically to the response buffer).
Note that this is only an HTTP/1.1 issue, AFAIK (I am pretty sure, though not positive, that WebSocket is not compatible with HTTP/1.0).
Thanks!
Greg
the recently added MHD_DAEMON_INFO_CURRENT_CONNECTIONS can return quite
outdated values in MHD_USE_THREAD_PER_CONNECTION mode. The reason is
that closed connections are collected in MHD_cleanup_connections, which
is called only from the select thread. In the
MHD_USE_THREAD_PER_CONNECTION mode that happens only after accepting an
connection -- therefore, the last connection is always not collected and
MHD_DAEMON_INFO_CURRENT_CONNECTIONS returns >= 1, even when there are no
connections. That makes it very unusable to detect whether all
connections have been handled.
Would you consider the attached patch, which calls
MHD_cleanup_connections whenever MHD_DAEMON_INFO_CURRENT_CONNECTIONS is
called? It makes MHD_DAEMON_INFO_CURRENT_CONNECTIONS slower, but the
returned value is much more accurate.
Cheers,
Milan Straka
I attach the first attempt on SO_REUSEPORT. The patch is available
either at https://github.com/foxik/libmicrohttpd/commit/9ce9422742e10458f87275ea202a982e00c2b88c
or attached. (It is against the version with
MHD_DAEMON_OPTION_CURRENT_CONNECTIONS, but I can rebase it to current
SVN HEAD if you want.)
It seems that a reasonably multiplatform way of detecting SO_REUSEPORT is
#ifdef SO_REPOSEPORT
which is used for example by Perl. For SO_EXCLUSIVEADDRUSE, the same
strategy seems to work too, according to Windows SDK headers and MinGW
WinAPI headers.
The current patch adds an option to allowing/disallowing address:port
reuse. One remark:
- currently both nonexisting SO_xxx and setsockopt failure are fatal and
MHD_start_daemon fails. That may be too harsh -- maybe the
MHD_OPTION_LISTENING_ADDRESS_REUSE should be only a hint.
Nevertheless, as one can freely not use
MHD_OPTION_LISTENING_ADDRESS_REUSE option, I chose the "fail on error"
behaviour.
Thanks,
cheers,
Milan Straka
Original patch modified to get rid of some redundant USE_DEBUG
checks, fix indentation, and #ifndef SO_REUSEPORT on Linux,
we try be #defining it to 15 ourselves.
Date: Wed, 29 Oct 2014 09:59:09 +0100
Subject: [PATCH 2/2] Add MHD_DAEMON_INFO_CURRENT_CONNECTIONS to
MHD_DaemonInfoType.
The MHD_DAEMON_INFO_CURRENT_CONNECTIONS returns number of current
connections handled by the daemon.
Useful after MHD_quiesce_daemon to find out whether all connections
have been served.
Date: Wed, 29 Oct 2014 09:17:42 +0100
Subject: [PATCH 1/2] Split daemon->max_connections to connections and
connection_limit.
In order to be able to return number of parallel connections, we need
to now both the actual number and the limit. Nevertheless, until now only
max_connections = connection_limit - connections
was kept. We now store both the connection_limit and connections.
>> Nevertheless, this means that there is an unhandled special case.
>> Consider MHD_USE_THREAD_PER_CONNECTION (either with or without
>> MHD_USE_POLL). Then wpipe is not created. After MHD_quiesce_daemon,
>> socket_fd = -1. Then, after the current select/poll in the
>> MHD_select_thread exits, there will be no fds to wait for (the socket_fd
>> is -1 and wpipe was not created), so the MHD_select_thread will be
>> busy-waiting for daemon->shutdown. Therefore, another condition should
>> be added to the beginning of MHD_quiesce_daemon:
>> current:
>>
>> MHD_quiesce_daemon (struct MHD_Daemon *daemon)
>> {
>> unsigned int i;
>> int ret;
>>
>> ret = daemon->socket_fd;
>> if (-1 == ret)
>> return -1;
>> if ( (-1 == daemon->wpipe[1]) &&
>> (0 != (daemon->options & MHD_USE_SELECT_INTERNALLY)) )
>> {
>> #if HAVE_MESSAGES
>> MHD_DLOG (daemon,
>> "Using MHD_quiesce_daemon in this mode requires MHD_USE_PIPE_FOR_SHUTDOWN\n");
>> #endif
>> return -1;
>> }
>>
>> new:
>>
>> if ( (-1 == daemon->wpipe[1]) &&
>> (0 != (daemon->options & (MHD_USE_SELECT_INTERNALLY | MHD_USE_THREAD_PER_CONNECTION )) ))
>>
>> Did I get it right?
>
> I don't think so. Note that the 'socket_fd' in the thread's
> select()/poll() call is not the (closed) listen socket, but the thread's
> TCP connection to the client. So the thread can still be terminated by
> calling shutdown() on that TCP connection socket.
> (The code is a bit confusing here, as both structs have a member called
> 'socket_fd').
(I am sorry I cannot make myself clear enough.)
I still think what I wrote holds. I am talking about the thread
executing MHD_select_thread, i.e., the one created in MHD_start_daemon_va.
It gets the daemon structure as parameter and accesses daemon->socket_fd.
I believe you are talking about threads executing MHD_handle_connection,
created in internal_add_connection.
The problem is that the listening thread might run out of FDs to wait
for. In the MHD_USE_THREAD_PER_CONNECTION case, the main listening
thread only selects/polls on two FDs -- daemon->socket_fd, and
daemon->wpipe[0]. And when daemon->socket_fd is closed and
daemon->wpipe[0] does not exist, problem occurs.
The same issue could happen when there is a thread_pool, but that is
taken care of in MHD_quiesce_daemon -- in this case, if there is no
wpipe, MHD_quiesce_daemon fails. What I am suggesting that this test
should also include the MHD_USE_THREAD_PER_CONNECTION case.
Cheers,
Milan Straka
