[xmltooling] 07/15: CPPXT-106 Apache Deadlock in load thread/lock thread

[Ferenc Wágner]

This is an automated email from the git hooks/post-receive script.

wferi pushed a commit to branch master
in repository xmltooling.

commit 4e832cb26d3fb46923b487ac53bfb600fc4a96b8
Author: Robert Crossfield <robcrossfield at gmail.com>
Date:   Tue Dec 20 11:04:06 2016 +1100

    CPPXT-106 Apache Deadlock in load thread/lock thread
    
    https://issues.shibboleth.net/jira/browse/CPPXT-106
    
    Under certain cirucmstances an Apache thread can
    cause us to do a Read then Write lock.  Diagnosed as being because
    of the lock dance in the ReloadableXMLFile::lock() call.
    
    Move the dance and updatingof m_filestamp to the reload thread,
    all the lock api needs to do is spot that things are out of date
    and kick the worker thread.  This might cause the worker to wake
    up too often but this is unlikely.
---
 xmltooling/util/ReloadableXMLFile.cpp | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/xmltooling/util/ReloadableXMLFile.cpp b/xmltooling/util/ReloadableXMLFile.cpp
index 7505616..4880577 100644
--- a/xmltooling/util/ReloadableXMLFile.cpp
+++ b/xmltooling/util/ReloadableXMLFile.cpp
@@ -287,7 +287,26 @@ void* ReloadableXMLFile::reload_fn(void* pv)
             r->m_reload_wait->timedwait(mutex.get(), r->m_reloadInterval);
         if (r->m_shutdown)
             break;
+		
+#ifdef WIN32
+        struct _stat stat_buf;
+        if (_stat(r->m_source.c_str(), &stat_buf) != 0)
+            continue;
+#else
+        struct stat stat_buf;
+        if (stat(r->m_source.c_str(), &stat_buf) != 0)
+            continue
+#endif
+        if (r->m_filestamp >= stat_buf.st_mtime)
+            continue;
 
+        // Elevate lock and recheck.
+        r->m_log.debug("timestamp of local resource changed, obtaining write lock");
+        r->m_lock->wrlock();
+        r->m_filestamp = stat_buf.st_mtime;
+        r->m_log.debug("timestamp of local resource changed, releasing write lock");
+        r->m_lock->unlock();
+		
         try {
             r->m_log.info("reloading %s resource...", r->m_local ? "local" : "remote");
             pair<bool,DOMElement*> ret = r->background_load();
@@ -340,20 +359,6 @@ Lockable* ReloadableXMLFile::lock()
         if (m_filestamp >= stat_buf.st_mtime)
             return this;
 
-        // Elevate lock and recheck.
-        m_log.debug("timestamp of local resource changed, elevating to a write lock");
-        m_lock->unlock();
-        m_lock->wrlock();
-        if (m_filestamp >= stat_buf.st_mtime) {
-            // Somebody else handled it, just downgrade.
-            m_log.debug("update of local resource handled by another thread, downgrading lock");
-            m_lock->unlock();
-            m_lock->rdlock();
-            return this;
-        }
-
-        // Update the timestamp regardless.
-        m_filestamp = stat_buf.st_mtime;
         if (m_reload_wait) {
             m_log.info("change detected, signaling reload thread...");
             m_reload_wait->signal();

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-shibboleth/xmltooling.git