- fixed logging during shutdown of snapperd to avoid core dumps

diff --git a/package/snapper.changes b/package/snapper.changes
index 4ef96b591ebe1163459f5244cb40842dd72931f6..8e2f5f32a9d28215ce4c47fc17691c0e42383b5b 100644 (file)
--- a/package/snapper.changes
+++ b/package/snapper.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon Jul 23 20:52:26 CEST 2018 - aschnell@suse.com
+
+- fixed logging during shutdown of snapperd to avoid core dumps
+  (bsc#1051922 and others)
+
 -------------------------------------------------------------------
 Tue May 09 13:52:46 CEST 2017 - aschnell@suse.com
 

diff --git a/server/Client.cc b/server/Client.cc
index 6d69d5edd1e46d8f36c01f3cf2862cf24ebc285a..dc7c69c4a9b936e6e87d32c29caabf8773d35872 100644 (file)
--- a/server/Client.cc
+++ b/server/Client.cc
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) [2012-2015] Novell, Inc.
- * Copyright (c) 2016 SUSE LLC
+ * Copyright (c) [2016,2018] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -68,11 +68,9 @@ Client::~Client()
        Snapshots& snapshots = snapper->getSnapshots();
 
        Snapshots::iterator snap = snapshots.find(number);
-       if (snap == snapshots.end())
-           throw IllegalSnapshotException();
-
-       for (unsigned int i = 0; i < use_count; ++i)
-           snap->umountFilesystemSnapshot(false);
+       if (snap != snapshots.end())
+           for (unsigned int i = 0; i < use_count; ++i)
+               snap->umountFilesystemSnapshot(false);
     }
 }
 
@@ -115,6 +113,7 @@ Client::delete_comparison(list<Comparison*>::iterator it)
     }
 
     delete *it;
+    *it = nullptr;
 }
 
 

diff --git a/snapper/Logger.cc b/snapper/Logger.cc
index 817a3ccaa8f17b39b4ed6ca6e36fcb7ef2bd13fc..d778ebae2bb975ceaa56ce00ff1a157e76198140 100644 (file)
--- a/snapper/Logger.cc
+++ b/snapper/Logger.cc
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) [2004-2013] Novell, Inc.
+ * Copyright (c) 2018 SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -32,15 +33,33 @@
 #include "snapper/AppUtil.h"
 
 
+#define LOG_FILENAME "/var/log/snapper.log"
+
+
 namespace snapper
 {
     using namespace std;
 
 
-    // Use a pointer to avoid a global destructor. Otherwise the Snapper
-    // destructor in Factory.cc can be called after when logging does not work
-    // anymore. TODO: nicer solution.
-    string* filename = new string("/var/log/snapper.log");
+    namespace
+    {
+
+       struct LoggerData
+       {
+           LoggerData() : filename(LOG_FILENAME), mutex() {}
+
+           string filename;
+           boost::mutex mutex;
+       };
+
+
+       // Use pointer to avoid a global destructor. Otherwise other global
+       // destructors using logging can cause errors.
+
+       LoggerData* logger_data = new LoggerData();
+
+    }
+
 
     LogDo log_do = NULL;
     LogQuery log_query = NULL;
@@ -69,11 +88,9 @@ namespace snapper
        string prefix = sformat("%s %s libsnapper(%d) %s(%s):%d", datetime(time(0), false, true).c_str(),
                                ln[level], getpid(), file, func, line);
 
-       static boost::mutex mutex;
-
-       boost::lock_guard<boost::mutex> lock(mutex);
+       boost::lock_guard<boost::mutex> lock(logger_data->mutex);
 
-       FILE* f = fopen(filename->c_str(), "ae");
+       FILE* f = fopen(logger_data->filename.c_str(), "ae");
        if (f)
        {
            string tmp = text;
@@ -137,8 +154,7 @@ namespace snapper
     void
     initDefaultLogger()
     {
-       delete filename;
-       filename = new string("/var/log/snapper.log");
+       logger_data->filename = LOG_FILENAME;
 
        if (geteuid())
        {
@@ -152,8 +168,7 @@ namespace snapper
            {
                memset(pwd.pw_passwd, 0, strlen(pwd.pw_passwd));
 
-               delete filename;
-               filename = new string(string(pwd.pw_dir) + "/.snapper.log");
+               logger_data->filename = string(pwd.pw_dir) + "/.snapper.log";
            }
        }