From 645e628626f4a3d4b662c067584b4efc6b5c70c5 Mon Sep 17 00:00:00 2001
From: William Brown <firstyear@redhat.com>
Date: Wed, 15 Mar 2017 10:46:38 +1000
Subject: [PATCH 5/5] Ticket 49171 - Nunc Stans incorrectly reports a timeout
Bug Description: In some cases nunc-stans would incorrectly report
and IO timeout.
Fix Description: Make the io output type volatile to prevent re-arranging
of the code. We then make timeout exclusive to read, write and signal.
Finally, we add an extra check into ns_handle_pr_read_ready that
asserts we truly have an idle timeout. It issues a warning now
instead if this scenario occurs, rather than closing the
connection.
https://pagure.io/389-ds-base/issue/49171
Author: wibrown
Review by: mreynolds (thanks!)
(cherry picked from commit c8ce1b32cc365174c8280111c2d55bba45f7949f)
---
ldap/servers/slapd/daemon.c | 15 +++++++++++----
src/nunc-stans/ns/ns_event_fw_event.c | 28 ++++++++++++++++------------
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index a37c8c6..6b3331d 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1970,11 +1970,18 @@ ns_handle_pr_read_ready(struct ns_job_t *job)
connection_release_nolock_ext(c, 1); /* release ref acquired when job was added */
if (CONN_NEEDS_CLOSING(c)) {
ns_handle_closure_nomutex(c);
+ /* We shouldn't need the c_idletimeout check here because of how libevent works.
+ * consider testing this and removing it oneday.
+ */
} else if (NS_JOB_IS_TIMER(ns_job_get_output_type(job))) {
- /* idle timeout */
- disconnect_server_nomutex_ext(c, c->c_connid, -1,
- SLAPD_DISCONNECT_IDLE_TIMEOUT, EAGAIN,
- 0 /* do not schedule closure, do it next */);
+ if (c->c_idletimeout > 0) {
+ /* idle timeout */
+ disconnect_server_nomutex_ext(c, c->c_connid, -1,
+ SLAPD_DISCONNECT_IDLE_TIMEOUT, EAGAIN,
+ 0 /* do not schedule closure, do it next */);
+ } else {
+ slapi_log_err(SLAPI_LOG_WARNING, "ns_handle_pr_read_ready", "Received idletime out with c->c_idletimeout as 0. Ignoring.\n");
+ }
ns_handle_closure_nomutex(c);
} else if ((connection_activity(c, maxthreads)) == -1) {
/* This might happen as a result of
diff --git a/src/nunc-stans/ns/ns_event_fw_event.c b/src/nunc-stans/ns/ns_event_fw_event.c
index 58dac28..3acbaf7 100644
--- a/src/nunc-stans/ns/ns_event_fw_event.c
+++ b/src/nunc-stans/ns/ns_event_fw_event.c
@@ -71,18 +71,22 @@ event_logger_cb(int severity, const char *msg)
static ns_job_type_t
event_flags_to_type(short events)
{
- ns_job_type_t job_type = 0;
- if (events & EV_READ) {
- job_type |= NS_JOB_READ;
- }
- if (events & EV_WRITE) {
- job_type |= NS_JOB_WRITE;
- }
- if (events & EV_TIMEOUT) {
- job_type |= NS_JOB_TIMER;
- }
- if (events & EV_SIGNAL) {
- job_type |= NS_JOB_SIGNAL;
+ /* The volatile here prevents gcc rearranging this code within the thread. */
+ volatile ns_job_type_t job_type = 0;
+
+ /* Either we timeout *or* we are a real event */
+ if (!(events & EV_TIMEOUT)) {
+ if (events & EV_READ) {
+ job_type |= NS_JOB_READ;
+ }
+ if (events & EV_WRITE) {
+ job_type |= NS_JOB_WRITE;
+ }
+ if (events & EV_SIGNAL) {
+ job_type |= NS_JOB_SIGNAL;
+ }
+ } else {
+ job_type = NS_JOB_TIMER;
}
return job_type;
}
--
2.9.3