From 4efe26cbd3ef0d85656bf00ac9e5bd41cc8a2a36 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 3 Nov 2015 09:12:52 -0500 Subject: [PATCH] shm_mq: Third attempt at fixing nowait behavior in shm_mq_receive. Commit a1480ec1d3bacb9acb08ec09f22bc25bc033115b purported to fix the problems with commit b2ccb5f4e6c81305386edb34daf7d1d1e1ee112a, but it didn't completely fix them. The problem is that the checks were performed in the wrong order, leading to a race condition. If the sender attached, sent a message, and detached after the receiver called shm_mq_get_sender and before the receiver called shm_mq_counterparty_gone, we'd incorrectly return SHM_MQ_DETACHED before all messages were read. Repair by reversing the order of operations, and add a long comment explaining why this new logic is (hopefully) correct. --- src/backend/storage/ipc/shm_mq.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/ipc/shm_mq.c b/src/backend/storage/ipc/shm_mq.c index 1d0657f3858..dafc8a7ca76 100644 --- a/src/backend/storage/ipc/shm_mq.c +++ b/src/backend/storage/ipc/shm_mq.c @@ -501,11 +501,26 @@ shm_mq_receive(shm_mq_handle *mqh, Size *nbytesp, void **datap, bool nowait) { if (nowait) { + int counterparty_gone; + + /* + * We shouldn't return at this point at all unless the sender + * hasn't attached yet. However, the correct return value depends + * on whether the sender is still attached. If we first test + * whether the sender has ever attached and then test whether the + * sender has detached, there's a race condition: a sender that + * attaches and detaches very quickly might fool us into thinking + * the sender never attached at all. So, test whether our + * counterparty is definitively gone first, and only afterwards + * check whether the sender ever attached in the first place. + */ + counterparty_gone = shm_mq_counterparty_gone(mq, mqh->mqh_handle); if (shm_mq_get_sender(mq) == NULL) { - if (shm_mq_counterparty_gone(mq, mqh->mqh_handle)) + if (counterparty_gone) return SHM_MQ_DETACHED; - return SHM_MQ_WOULD_BLOCK; + else + return SHM_MQ_WOULD_BLOCK; } } else if (!shm_mq_wait_internal(mq, &mq->mq_sender, mqh->mqh_handle)