Details
-
Bug
-
Resolution: Fixed
-
1.6.0 beta4
-
None
-
Operating System: All
Platform: All
-
Untriaged
Description
Bug found by Jayesh
I would like you to take a look at one more thing – in ed4c1b14, you have the following change in thread.c:
@@ -394,7 +396,7 @@ void notify_io_complete(const void *cookie, ENGINE_ERROR_CODE status)
LOCK_THREAD(thr);
- if (thr != conn->thread || conn->state == conn_closing) {
+ if (thr != conn->thread || conn->state == conn_closing || !conn->ewouldblock) { UNLOCK_THREAD(thr); return; }
In the case where the engine returns EWOULDBLOCK, this change introduces a race between the engine’s background fetcher thread checking conn->ewouldblock inside notify_io_complete() and the original thread setting conn->ewouldblock after it gets EWOULDBLOCK back from the engine. Even the current binary implementation is affected by this bug.
There are also other properties of the connection that we modify inside notify_io_complete(), like state, aiostate etc. Since we do not have locking at the connection level, what do you think about queuing notify_io_complete() to be called in the thread that owns the connection to avoid these kind of issues in the future?