diff options
| author | Robert Haas <rhaas@postgresql.org> | 2015-11-03 09:12:52 -0500 |
|---|---|---|
| committer | Robert Haas <rhaas@postgresql.org> | 2015-11-03 09:12:52 -0500 |
| commit | 4efe26cbd3ef0d85656bf00ac9e5bd41cc8a2a36 (patch) | |
| tree | 85aa24973256684c420378fe4f3264a3c9bfc01d /src | |
| parent | 0279f62fdca7ff19ef34a836f5201935dc7f627c (diff) | |
| download | postgresql-4efe26cbd3ef0d85656bf00ac9e5bd41cc8a2a36.tar.gz postgresql-4efe26cbd3ef0d85656bf00ac9e5bd41cc8a2a36.zip | |
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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/storage/ipc/shm_mq.c | 19 |
1 files 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) |