View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003126 | libmicrohttpd | external API | public | 2013-11-22 21:20 | 2013-12-04 18:54 |
| Reporter | matt.holiday | Assigned To | Christian Grothoff | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | Linux | ||||
| Product Version | Git master | ||||
| Target Version | 0.9.32 | Fixed in Version | 0.9.32 | ||
| Summary | 0003126: Issues with new socket suspend/resume API | ||||
| Description | Either sockets don't wake up or they don't suspend properly; patch attached based on svn 30760 | ||||
| Steps To Reproduce | Use new experimental suspend/resume API, primarily in the thread pool case Problems found both using select and epoll | ||||
| Additional Information | Changes: internal.h 1. define MHD_EPOLL_STATE_SUSPENDED 2. fix definition of MHD_TLS_CONNECTION_INIT connection.c 1. check MHD_EPOLL_STATE_SUSPENDED in 4 places before EDLL_insert 2. also check it before doing an epoll add in the update function daemon.c 1. mutex around DLL_remove etc. 2. check MHD_USE_EPOLL_LINUX_ONLY in suspend/resume epoll logic 3. set/clear MHD_EPOLL_STATE_SUSPENDED there 4. panic on failure to re-add fd to epoll in resume 5. on worker create, give each their own wpipe 6. use mutex in clean, and set state to MHD_CONNECTION_IN_CLEANUP 7. in shutdown, signal to the worker pipes after setting shutdown flag 8. in shutdown, don't check fd==-1 for worker epoll 9. in shutdown, delete the worker pipes | ||||
| Tags | No tags attached. | ||||
| Attached Files | libmicrohttpd.diff (10,545 bytes)
Index: src/microhttpd/internal.h
===================================================================
--- src/microhttpd/internal.h (revision 30839)
+++ src/microhttpd/internal.h (working copy)
@@ -116,8 +116,12 @@
/**
* Is this connection currently in the 'epoll' set?
*/
- MHD_EPOLL_STATE_IN_EPOLL_SET = 8
+ MHD_EPOLL_STATE_IN_EPOLL_SET = 8,
+ /**
+ * Is this connection currently suspended?
+ */
+ MHD_EPOLL_STATE_SUSPENDED = 16
};
@@ -452,7 +456,7 @@
* Handshake messages will be processed in this state & while
* in the 'MHD_TLS_HELLO_REQUEST' state
*/
- MHD_TLS_CONNECTION_INIT = MHD_CONNECTION_CLOSED + 1
+ MHD_TLS_CONNECTION_INIT = MHD_CONNECTION_IN_CLEANUP + 1
};
Index: src/microhttpd/connection.c
===================================================================
--- src/microhttpd/connection.c (revision 30839)
+++ src/microhttpd/connection.c (working copy)
@@ -2474,6 +2474,7 @@
{
case MHD_EVENT_LOOP_INFO_READ:
if ( (0 != (connection->epoll_state & MHD_EPOLL_STATE_READ_READY)) &&
+ (0 == (connection->epoll_state & MHD_EPOLL_STATE_SUSPENDED)) &&
(0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL)) )
{
EDLL_insert (daemon->eready_head,
@@ -2485,6 +2486,7 @@
case MHD_EVENT_LOOP_INFO_WRITE:
if ( (connection->read_buffer_size > connection->read_buffer_offset) &&
(0 != (connection->epoll_state & MHD_EPOLL_STATE_READ_READY)) &&
+ (0 == (connection->epoll_state & MHD_EPOLL_STATE_SUSPENDED)) &&
(0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL)) )
{
EDLL_insert (daemon->eready_head,
@@ -2493,6 +2495,7 @@
connection->epoll_state |= MHD_EPOLL_STATE_IN_EREADY_EDLL;
}
if ( (0 != (connection->epoll_state & MHD_EPOLL_STATE_WRITE_READY)) &&
+ (0 == (connection->epoll_state & MHD_EPOLL_STATE_SUSPENDED)) &&
(0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL)) )
{
EDLL_insert (daemon->eready_head,
@@ -2504,7 +2507,8 @@
case MHD_EVENT_LOOP_INFO_BLOCK:
/* we should look at this connection again in the next iteration
of the event loop, as we're waiting on the application */
- if (0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL))
+ if ( (0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL) &&
+ (0 == (connection->epoll_state & MHD_EPOLL_STATE_SUSPENDED))) )
{
EDLL_insert (daemon->eready_head,
daemon->eready_tail,
@@ -2539,6 +2543,7 @@
if ( (0 != (daemon->options & MHD_USE_EPOLL_LINUX_ONLY)) &&
(0 == (connection->epoll_state & MHD_EPOLL_STATE_IN_EPOLL_SET)) &&
+ (0 == (connection->epoll_state & MHD_EPOLL_STATE_SUSPENDED)) &&
( (0 == (connection->epoll_state & MHD_EPOLL_STATE_WRITE_READY)) ||
( (0 == (connection->epoll_state & MHD_EPOLL_STATE_READ_READY)) &&
( (MHD_EVENT_LOOP_INFO_READ == connection->event_loop_info) ||
Index: src/microhttpd/daemon.c
===================================================================
--- src/microhttpd/daemon.c (revision 30839)
+++ src/microhttpd/daemon.c (working copy)
@@ -1398,6 +1398,9 @@
daemon = connection->daemon;
if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
MHD_PANIC ("Cannot suspend connections in THREAD_PER_CONNECTION mode!\n");
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (0 != pthread_mutex_lock (&daemon->cleanup_connection_mutex)) )
+ MHD_PANIC ("Failed to acquire cleanup mutex\n");
DLL_remove (daemon->connections_head,
daemon->connections_tail,
connection);
@@ -1410,22 +1413,29 @@
daemon->manual_timeout_tail,
connection);
#if EPOLL_SUPPORT
- if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL))
- {
- EDLL_remove (daemon->eready_head,
- daemon->eready_tail,
- connection);
- }
- if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EPOLL_SET))
- {
- if (0 != epoll_ctl (daemon->epoll_fd,
- EPOLL_CTL_DEL,
- connection->socket_fd,
- NULL))
- MHD_PANIC ("Failed to remove FD from epoll set\n");
- connection->epoll_state &= ~MHD_EPOLL_STATE_IN_EPOLL_SET;
- }
+ if (0 != (daemon->options & MHD_USE_EPOLL_LINUX_ONLY))
+ {
+ if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL))
+ {
+ EDLL_remove (daemon->eready_head,
+ daemon->eready_tail,
+ connection);
+ }
+ if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EPOLL_SET))
+ {
+ if (0 != epoll_ctl (daemon->epoll_fd,
+ EPOLL_CTL_DEL,
+ connection->socket_fd,
+ NULL))
+ MHD_PANIC ("Failed to remove FD from epoll set\n");
+ connection->epoll_state &= ~MHD_EPOLL_STATE_IN_EPOLL_SET;
+ }
+ connection->epoll_state |= MHD_EPOLL_STATE_SUSPENDED;
+ }
#endif
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (0 != pthread_mutex_unlock (&daemon->cleanup_connection_mutex)) )
+ MHD_PANIC ("Failed to release cleanup mutex\n");
}
@@ -1460,33 +1470,30 @@
daemon->manual_timeout_tail,
connection);
#if EPOLL_SUPPORT
- if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL))
- {
- EDLL_insert (daemon->eready_head,
- daemon->eready_tail,
- connection);
- }
- else
- {
- struct epoll_event event;
+ if (0 != (daemon->options & MHD_USE_EPOLL_LINUX_ONLY))
+ {
+ if (0 != (connection->epoll_state & MHD_EPOLL_STATE_IN_EREADY_EDLL))
+ {
+ EDLL_insert (daemon->eready_head,
+ daemon->eready_tail,
+ connection);
+ }
+ else
+ {
+ struct epoll_event event;
- event.events = EPOLLIN | EPOLLOUT | EPOLLET;
- event.data.ptr = connection;
- if (0 != epoll_ctl (daemon->epoll_fd,
- EPOLL_CTL_ADD,
- connection->socket_fd,
- &event))
- {
-#if HAVE_MESSAGES
- MHD_DLOG (daemon,
- "Call to epoll_ctl failed: %s\n",
- STRERROR (errno));
-#endif
- /* and now, good luck with this... */
- }
- else
- connection->epoll_state |= MHD_EPOLL_STATE_IN_EPOLL_SET;
+ event.events = EPOLLIN | EPOLLOUT | EPOLLET;
+ event.data.ptr = connection;
+ if (0 != epoll_ctl (daemon->epoll_fd,
+ EPOLL_CTL_ADD,
+ connection->socket_fd,
+ &event))
+ MHD_PANIC ("Failed to add FD to epoll set\n");
+ else
+ connection->epoll_state |= MHD_EPOLL_STATE_IN_EPOLL_SET;
}
+ connection->epoll_state &= ~MHD_EPOLL_STATE_SUSPENDED;
+ }
#endif
if ( (-1 != daemon->wpipe[1]) &&
(1 != WRITE (daemon->wpipe[1], "n", 1)) )
@@ -3581,6 +3588,38 @@
d->worker_pool_size = 0;
d->worker_pool = NULL;
+ if ( (use_pipe) &&
+#ifdef WINDOWS
+ (0 != SOCKETPAIR (AF_INET, SOCK_STREAM, IPPROTO_TCP, d->wpipe))
+#else
+ (0 != PIPE (d->wpipe))
+#endif
+ )
+ {
+#if HAVE_MESSAGES
+ MHD_DLOG (daemon,
+ "Failed to create worker control pipe: %s\n",
+ STRERROR (errno));
+#endif
+ goto thread_failed;
+ }
+#ifndef WINDOWS
+ if ( (0 == (flags & MHD_USE_POLL)) &&
+ (1 == use_pipe) &&
+ (d->wpipe[0] >= FD_SETSIZE) )
+ {
+#if HAVE_MESSAGES
+ MHD_DLOG (daemon,
+ "file descriptor for worker control pipe exceeds maximum value\n");
+#endif
+ if (0 != CLOSE (d->wpipe[0]))
+ MHD_PANIC ("close failed\n");
+ if (0 != CLOSE (d->wpipe[1]))
+ MHD_PANIC ("close failed\n");
+ goto thread_failed;
+ }
+#endif
+
/* Divide available connections evenly amongst the threads.
* Thread indexes in [0, leftover_conns) each get one of the
* leftover connections. */
@@ -3678,6 +3717,9 @@
MHD_connection_close (pos,
MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (0 != pthread_mutex_lock (&daemon->cleanup_connection_mutex)) )
+ MHD_PANIC ("Failed to acquire cleanup mutex\n");
if (pos->connection_timeout == pos->daemon->connection_timeout)
XDLL_remove (daemon->normal_timeout_head,
daemon->normal_timeout_tail,
@@ -3690,9 +3732,13 @@
daemon->connections_tail,
pos);
pos->event_loop_info = MHD_EVENT_LOOP_INFO_CLEANUP;
+ pos->state = MHD_CONNECTION_IN_CLEANUP;
DLL_insert (daemon->cleanup_head,
daemon->cleanup_tail,
pos);
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (0 != pthread_mutex_unlock (&daemon->cleanup_connection_mutex)) )
+ MHD_PANIC ("Failed to release cleanup mutex\n");
}
@@ -3794,10 +3840,14 @@
{
daemon->worker_pool[i].shutdown = MHD_YES;
daemon->worker_pool[i].socket_fd = -1;
+ if (-1 != daemon->wpipe[1])
+ {
+ if (1 != WRITE (daemon->worker_pool[i].wpipe[1], "e", 1))
+ MHD_PANIC ("failed to signal shutdownn via pipe");
+ }
#if EPOLL_SUPPORT
if ( (0 != (daemon->options & MHD_USE_EPOLL_LINUX_ONLY)) &&
- (-1 != daemon->worker_pool[i].epoll_fd) &&
- (-1 == fd) )
+ (-1 != daemon->worker_pool[i].epoll_fd) )
epoll_shutdown (&daemon->worker_pool[i]);
#endif
}
@@ -3839,6 +3889,13 @@
MHD_PANIC ("Failed to join a thread\n");
close_all_connections (&daemon->worker_pool[i]);
pthread_mutex_destroy (&daemon->worker_pool[i].cleanup_connection_mutex);
+ if (-1 != daemon->wpipe[1])
+ {
+ if (0 != CLOSE (daemon->worker_pool[i].wpipe[0]))
+ MHD_PANIC ("close failed\n");
+ if (0 != CLOSE (daemon->worker_pool[i].wpipe[1]))
+ MHD_PANIC ("close failed\n");
+ }
#if EPOLL_SUPPORT
if ( (-1 != daemon->worker_pool[i].epoll_fd) &&
(0 != CLOSE (daemon->worker_pool[i].epoll_fd)) )
| ||||
|
|
Ok, just saw this now. Parts of the patch have been applied to SVN HEAD. Not applied are 5-9, as we must not have worker-pipes per worker; instead the main signalling pipe is used differently now to ensure all works get the signal. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-11-22 21:20 | matt.holiday | New Issue | |
| 2013-11-22 21:20 | matt.holiday | File Added: libmicrohttpd.diff | |
| 2013-11-29 12:41 | Christian Grothoff | Note Added: 0007745 | |
| 2013-11-29 12:41 | Christian Grothoff | Status | new => resolved |
| 2013-11-29 12:41 | Christian Grothoff | Fixed in Version | => 0.9.32 |
| 2013-11-29 12:41 | Christian Grothoff | Resolution | open => fixed |
| 2013-11-29 12:41 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2013-11-29 12:41 | Christian Grothoff | Product Version | => Git master |
| 2013-11-29 12:41 | Christian Grothoff | Target Version | => 0.9.32 |
| 2013-12-04 18:54 | Christian Grothoff | Status | resolved => closed |
| 2024-01-21 13:24 | Christian Grothoff | Category | libmicrohttpd API => external API |
