diff options
| author | Christian Grothoff <christian@grothoff.org> | 2014-10-28 08:55:50 +0000 |
|---|---|---|
| committer | Christian Grothoff <christian@grothoff.org> | 2014-10-28 08:55:50 +0000 |
| commit | 4d1fd8c61d25b0363048785119de246e148806d3 (patch) | |
| tree | 104726568bfd03ffd44a9430478298a2906bc380 | |
| parent | 5ad10328b31f817c3aad60acfe72bbaebb7570ec (diff) | |
| download | libmicrohttpd-4d1fd8c61d25b0363048785119de246e148806d3.tar.gz libmicrohttpd-4d1fd8c61d25b0363048785119de246e148806d3.zip | |
Milan is right.
>> 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
| -rw-r--r-- | src/microhttpd/daemon.c | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c index f942aa6b..2749fb1a 100644 --- a/src/microhttpd/daemon.c +++ b/src/microhttpd/daemon.c | |||
| @@ -2773,7 +2773,7 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon) | |||
| 2773 | if (MHD_INVALID_SOCKET == ret) | 2773 | if (MHD_INVALID_SOCKET == ret) |
| 2774 | return MHD_INVALID_SOCKET; | 2774 | return MHD_INVALID_SOCKET; |
| 2775 | if ( (MHD_INVALID_PIPE_ == daemon->wpipe[1]) && | 2775 | if ( (MHD_INVALID_PIPE_ == daemon->wpipe[1]) && |
| 2776 | (0 != (daemon->options & MHD_USE_SELECT_INTERNALLY)) ) | 2776 | (0 != (daemon->options & (MHD_USE_SELECT_INTERNALLY | MHD_USE_THREAD_PER_CONNECTION))) ) |
| 2777 | { | 2777 | { |
| 2778 | #if HAVE_MESSAGES | 2778 | #if HAVE_MESSAGES |
| 2779 | MHD_DLOG (daemon, | 2779 | MHD_DLOG (daemon, |