[cpp-sp] branch main updated: Reimplement file cleanup on older class.

Scott Cantor cantor.2 at osu.edu
Tue Jun 24 15:40:28 UTC 2025


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

scantor pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=7cb7357f5a0d6f49eec41d7df4718860c7e7ab0d

The following commit(s) were added to refs/heads/main by this push:
     new 7cb7357f Reimplement file cleanup on older class.
7cb7357f is described below

commit 7cb7357f5a0d6f49eec41d7df4718860c7e7ab0d
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jun 24 11:40:24 2025 -0400

    Reimplement file cleanup on older class.
---
 shibsp/session/impl/FilesystemSessionCache.cpp | 90 +++++++++-----------------
 shibsp/util/DirectoryWalker.cpp                | 11 ++--
 shibsp/util/DirectoryWalker.h                  |  4 +-
 3 files changed, 39 insertions(+), 66 deletions(-)

diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index 84f6065f..5beff1df 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -25,14 +25,11 @@
 #include "session/AbstractSessionCache.h"
 #include "logging/Category.h"
 #include "util/Date.h"
+#include "util/DirectoryWalker.h"
 #include "util/Misc.h"
 #include "util/PathResolver.h"
 
-#include <cstdio>
 #include <fstream>
-#ifdef HAVE_CXX17
-# include <filesystem>
-#endif
 
 #include <fcntl.h>
 
@@ -77,15 +74,16 @@ namespace {
         void cache_remove(SPRequest* request, const char* key);
 
     private:
-#ifdef HAVE_CXX17
         static void* file_cleanup_fn(void*);
-        condition_variable m_file_cleanup_wait;
-        thread m_file_cleanup_thread;
-#endif
+        static void file_cleanup_callback(const char* pathname, const char* filename, struct stat& stat_buf, void* data);
+        
         Category& m_spilog;
         string m_dir;
         duthomhas::csprng m_rng;
         time_t m_cleanupInterval;
+        unsigned int m_fileTimeout;
+        condition_variable m_file_cleanup_wait;
+        thread m_file_cleanup_thread;
     };
 
     static const char CACHE_DIRECTORY_PROP_NAME[] = "cacheDirectory";
@@ -145,11 +143,7 @@ FilesystemSessionCache::FilesystemSessionCache(const ptree& pt)
 
     m_cleanupInterval = getUnsignedInt(FILE_CLEANUP_INTERVAL_PROP_NAME, FILE_CLEANUP_INTERVAL_PROP_DEFAULT);
     if (m_cleanupInterval) {
-#ifndef HAVE_CXX17
-        m_spilog.warn("file cleanup thread disabled, C++17 build required");
-        m_cleanupInterval = 0;
-        return;
-#endif
+        m_fileTimeout = getUnsignedInt(FILE_TIMEOUT_PROP_NAME, FILE_TIMEOUT_PROP_DEFAULT);
     }
     else {
         m_spilog.info("%s was zero, disabling file cleanup thread", FILE_CLEANUP_INTERVAL_PROP_NAME);
@@ -360,8 +354,6 @@ void FilesystemSessionCache::cache_remove(SPRequest* request, const char* key)
     }
 }
 
-#ifdef HAVE_CXX17
-
 void* FilesystemSessionCache::file_cleanup_fn(void* p)
 {
     FilesystemSessionCache* pcache = reinterpret_cast<FilesystemSessionCache*>(p);
@@ -374,7 +366,6 @@ void* FilesystemSessionCache::file_cleanup_fn(void* p)
 #endif
 
     // Load our configuration details...
-    unsigned int fileTimeout = pcache->getUnsignedInt(FILE_TIMEOUT_PROP_NAME, FILE_TIMEOUT_PROP_DEFAULT);
     string cleanupTracker = pcache->m_dir + pcache->getString(
         FILE_CLEANUP_TRACKING_FILE_PROP_NAME, FILE_CLEANUP_TRACKING_FILE_PROP_DEFAULT);
 
@@ -407,7 +398,7 @@ void* FilesystemSessionCache::file_cleanup_fn(void* p)
     unique_lock lock(internal_mutex);
 
     pcache->m_spilog.info("file cleanup thread started...run every %u secs, purge after %u seconds of disuse",
-        pcache->m_cleanupInterval, fileTimeout);
+        pcache->m_cleanupInterval, pcache->m_fileTimeout);
 
     while (!pcache->isShutdown()) {
         pcache->m_file_cleanup_wait.wait_for(lock, chrono::seconds(pcache->m_cleanupInterval));
@@ -439,48 +430,9 @@ void* FilesystemSessionCache::file_cleanup_fn(void* p)
             continue;
         }
 
-        // While there are warnings all over the place about the C++17 filesystem APIs,
-        // I am suspecting that for our purposes the issues won't matter much.
-        // Admittedly, sticking a symlink into the directory could fool this into
-        // blowing away lots of content the agent can write to. Maybe don't do that?
-
         try {
-            for (auto& dir_entry : filesystem::directory_iterator{pcache->m_dir}) {
-                if (!dir_entry.is_regular_file() || dir_entry.path() == cleanupTracker) {
-                    continue;
-                }
-                
-                auto filename = dir_entry.path().filename();
-                if (filename.string().size() != 32) {
-                    pcache->m_spilog.warn("skipping unexpected filename (%s)", filename.c_str());
-                    continue;
-                }
-
-                // C++17 is indeed so messy that it didn't specify the filesystem epoch be
-                // the system clock, so it's non-portable to convert the last_write_time
-                // into a time_t. So we'll have to use our helper via stat to obtain the
-                // timestamp.
-
-                auto* pathname = dir_entry.path().c_str();
-                time_t modified = FileSupport::getModificationTime(pathname);
-                if (modified > 0 && now - modified > fileTimeout) {
-#ifdef WIN32
-                    if (_wremove(pathname) == 0) {
-                        pcache->m_spilog.info("removed stale session file (%S)", pathname);
-                    }
-                    else {
-                        pcache->m_spilog.info("error removing stale session file (%S), errno=%d", pathname, errno);
-                    }
-#else
-                    if (std::remove(pathname) == 0) {
-                        pcache->m_spilog.info("removed stale session file (%s)", pathname);
-                    }
-                    else {
-                        pcache->m_spilog.info("error removing stale session file (%s), errno=%d", pathname, errno);
-                    }
-#endif
-                }
-            } 
+            DirectoryWalker dirWalker(pcache->m_spilog, pcache->m_dir.c_str());
+            dirWalker.walk(&file_cleanup_callback, p);
         }
         catch (const exception& e) {
             pcache->m_spilog.error("caught exception during cleanup: %s", e.what());
@@ -494,4 +446,24 @@ void* FilesystemSessionCache::file_cleanup_fn(void* p)
     return nullptr;
 }
 
-#endif
+void FilesystemSessionCache::file_cleanup_callback(
+    const char* pathname, const char* filename, struct stat& stat_buf, void* data
+    )
+{
+    FilesystemSessionCache* pcache = reinterpret_cast<FilesystemSessionCache*>(data);
+
+    if (strlen(filename) != 32) {
+        pcache->m_spilog.warn("skipping unexpected filename (%s)", filename);
+        return;
+    }
+
+    if (stat_buf.st_mtime > 0 && time(nullptr) - stat_buf.st_mtime > pcache->m_fileTimeout) {
+        if (std::remove(pathname) == 0) {
+            pcache->m_spilog.info("removed stale session file (%s)", filename);
+        }
+        else {
+            pcache->m_spilog.info("error removing stale session file (%s), errno=%d", filename, errno);
+        }
+    }
+
+}
\ No newline at end of file
diff --git a/shibsp/util/DirectoryWalker.cpp b/shibsp/util/DirectoryWalker.cpp
index 8ee56f5a..66be775c 100644
--- a/shibsp/util/DirectoryWalker.cpp
+++ b/shibsp/util/DirectoryWalker.cpp
@@ -63,7 +63,7 @@ void DirectoryWalker::_walk(
     HANDLE h = FindFirstFileA(searchpath.c_str(), &f);
     if (h == INVALID_HANDLE_VALUE) {
         if (GetLastError() != ERROR_FILE_NOT_FOUND)
-            m_log.warn("Unable to open directory (%s)", path);
+            m_log.warn("unable to open directory (%s)", path);
         else
             m_log.debug("no matching entries in directory (%s)", path);
         return;
@@ -86,7 +86,7 @@ void DirectoryWalker::_walk(
             struct stat stat_buf;
             if (stat(fullname.c_str(), &stat_buf) == 0) {
                 m_log.debug("invoking callback for file (%s)", fullname.c_str());
-                callback_fn(fullname.c_str(), stat_buf, callback_data);
+                callback_fn(fullname.c_str(), f.cFileName, stat_buf, callback_data);
             }
             else {
                 m_log.warn("unable to access (%s)", fullname.c_str());
@@ -97,15 +97,16 @@ void DirectoryWalker::_walk(
 #else
     DIR* d = opendir(path);
     if (!d) {
-        m_log.warn("Unable to open directory (%s)", path);
+        m_log.warn("unable to open directory (%s)", path);
         return;
     }
     char dir_buf[sizeof(struct dirent) + PATH_MAX];
     struct dirent* ent = (struct dirent*)dir_buf;
     struct dirent* entptr = nullptr;
     while (readdir_r(d, ent, &entptr) == 0 && entptr) {
-        if (!strcmp(entptr->d_name, ".") || !strcmp(entptr->d_name, ".."))
+        if (!strcmp(entptr->d_name, ".") || !strcmp(entptr->d_name, "..")) {
             continue;
+        }
         else if (startsWith || endsWith) {
             string fname(entptr->d_name);
             if ((startsWith && !boost::algorithm::starts_with(fname, startsWith)) ||
@@ -130,7 +131,7 @@ void DirectoryWalker::_walk(
         }
         else {
             m_log.debug("invoking callback for file (%s)", fullname.c_str());
-            callback_fn(fullname.c_str(), stat_buf, callback_data);
+            callback_fn(fullname.c_str(), entptr->d_name, stat_buf, callback_data);
         }
     }
     closedir(d);
diff --git a/shibsp/util/DirectoryWalker.h b/shibsp/util/DirectoryWalker.h
index d64d9625..6e540e09 100644
--- a/shibsp/util/DirectoryWalker.h
+++ b/shibsp/util/DirectoryWalker.h
@@ -53,8 +53,8 @@ namespace shibsp {
 
         virtual ~DirectoryWalker();
 
-        /** Callback function, passed the file pathname, stat buffer, and optional callback data. */
-        typedef void (*DirectoryWalkerCallback)(const char* pathname, struct stat& stat_buf, void* data);
+        /** Callback function, passed the path and file names, stat buffer, and optional callback data. */
+        typedef void (*DirectoryWalkerCallback)(const char* pathname, const char* filename, struct stat& stat_buf, void* data);
 
         /**
          * Perform a depth-first traversal of the directory.

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list