From 7c49988ebf5c176cadd4a9e287e443d49a2cdeec Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 17 Jul 2013 01:39:24 +0000 Subject: document ioq and mog_{mgmt,http}_drop interaction safety I needed to spend time to convince myself this was safe, so leave a note to others (and future self) in case there is cause for concern. Basically, this is highly dependent on our overall one-shot-based concurrency model and safe as long as basic rules are followed. --- http.c | 9 ++++++++- ioq.c | 11 +++++++++++ mgmt.c | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/http.c b/http.c index 19cf9b7..cc802c3 100644 --- a/http.c +++ b/http.c @@ -122,7 +122,14 @@ void mog_http_unlink_ftmp(struct mog_http *http) file->tmppath, http->svc->docroot); } -/* called if epoll/kevent is out-of-space */ +/* + * called only if epoll/kevent is out-of-space + * Note: it's impossible for this to be called while this mfd is + * inside an ioq SIMPLEQ, however mfd->ioq_blocked /may/ be true when + * this function is called. We could add validation code to ensure + * this remains true, but that would increase the size of "struct mog_fd", + * so we will rely on this comment instead. + */ void mog_http_drop(struct mog_fd *mfd) { struct mog_http *http = &mfd->as.http; diff --git a/ioq.c b/ioq.c index 9b0bd9a..e88d7f3 100644 --- a/ioq.c +++ b/ioq.c @@ -48,6 +48,11 @@ static inline void ioq_set_contended(struct mog_ioq *ioq) * This is like sem_trywait. Each thread is only allowed to acquire * one ioq at once. * + * If this returns false, the caller _must_ return MOG_NEXT_IGNORE to + * prevent the mfd from being added to an epoll/kqueue watch list. + * Adding the mfd to an epoll/kqueue watch list in the same thread/context + * where this function returns true is a guaranteed bug. + * * client_mfd is the client socket, not the open (regular) file */ bool mog_ioq_ready(struct mog_ioq *ioq, struct mog_fd *client_mfd) @@ -115,6 +120,12 @@ void mog_ioq_next(struct mog_ioq *check_ioq) /* wake up the next sleeper on this queue */ if (client_mfd) mog_activeq_push(mog_ioq_current->svc->queue, client_mfd); + /* + * We may not touch or use client_mfd here anymore. Another + * thread may already have it. In the worst case, it's been + * closed due to epoll/kqueue running out-of-space and another + * system call (open/accept) may have already reused the FD + */ mog_ioq_current = NULL; } diff --git a/mgmt.c b/mgmt.c index 42d6778..6a0a12a 100644 --- a/mgmt.c +++ b/mgmt.c @@ -83,7 +83,7 @@ MOG_NOINLINE static void mgmt_close(struct mog_fd *mfd) mog_fd_put(mfd); } -/* called if epoll/kevent is out-of-space */ +/* called only if epoll/kevent is out-of-space (see mog_http_drop) */ void mog_mgmt_drop(struct mog_fd *mfd) { struct mog_mgmt *mgmt = &mfd->as.mgmt; -- cgit v1.2.3-24-ge0c7