Merge pull request #415 from aschnell/master

- fixed logging during shutdown

Author: aschnell

VERSION

@@ -1 +1 @@
-0.5.5
+0.5.6

package/snapper.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Jul 23 20:52:26 CEST 2018 - aschnell@suse.com
+
+- fixed logging during shutdown of snapperd to avoid core dumps
+  (bsc#1096401 and others)
+- version 0.5.6
+
 -------------------------------------------------------------------
 Mon May 28 10:44:49 CEST 2018 - aschnell@suse.com
 

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;
 }
 
 

server/MetaSnapper.cc

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) [2012-2015] Novell, Inc.
+ * Copyright (c) 2018 SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -103,6 +104,7 @@ MetaSnapper::MetaSnapper(ConfigInfo& config_info)
 MetaSnapper::~MetaSnapper()
 {
     delete snapper;
+    snapper = nullptr;
 }
 
 
@@ -172,7 +174,7 @@ void
 MetaSnapper::unload()
 {
     delete snapper;
-    snapper = NULL;
+    snapper = nullptr;
 }
 
 
@@ -198,6 +200,14 @@ MetaSnappers::init()
 }
 
 
+void
+MetaSnappers::unload()
+{
+    for (MetaSnapper& meta_snapper : entries)
+	meta_snapper.unload();
+}
+
+
 MetaSnappers::iterator
 MetaSnappers::find(const string& config_name)
 {

server/MetaSnapper.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) [2012-2015] Novell, Inc.
+ * Copyright (c) 2018 SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -123,6 +124,8 @@ class MetaSnappers
 
     void init();
 
+    void unload();
+
     typedef list<MetaSnapper>::iterator iterator;
     typedef list<MetaSnapper>::const_iterator const_iterator;
 

server/snapperd.cc

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) [2012-2015] Novell, Inc.
+ * Copyright (c) 2018 SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -286,5 +287,7 @@ main(int argc, char** argv)
 
     y2mil("Exiting");
 
+    meta_snappers.unload();
+
     return 0;
 }

snapper/Log.cc

@@ -64,6 +64,7 @@ namespace snapper
     {
 	callLogDo(level, *component, file, line, func, stream->str());
 	delete stream;
+	stream = nullptr;
     }
 
 }

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";
 	    }
 	}
 

snapper/Snapper.cc

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) [2011-2015] Novell, Inc.
- * Copyright (c) 2016 SUSE LLC
+ * Copyright (c) [2016,2018] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -147,7 +147,10 @@ namespace snapper
 	}
 
 	delete filesystem;
+	filesystem = nullptr;
+
 	delete config_info;
+	config_info = nullptr;
     }
 
 

snapper/SnapperTmpl.h

@@ -23,7 +23,7 @@
 #ifndef SNAPPER_SNAPPER_TMPL_H
 #define SNAPPER_SNAPPER_TMPL_H
 
-#include <functional>
+
 #include <ostream>
 #include <fstream>
 #include <sstream>
@@ -32,6 +32,7 @@
 
 #include "snapper/AppUtil.h"
 
+
 namespace snapper
 {
     using std::string;
@@ -47,6 +48,7 @@ namespace snapper
 	return num_str.str();
     }
 
+
     template<class Num> string hexString(Num number)
     {
 	static_assert(std::is_integral<Num>::value, "not integral");
@@ -57,13 +59,15 @@ namespace snapper
 	return num_str.str();
     }
 
+
     template<class Value> void operator>>(const string& d, Value& v)
     {
 	std::istringstream Data(d);
 	classic(Data);
 	Data >> v;
     }
 
+
     template<class Value> std::ostream& operator<<( std::ostream& s, const std::list<Value>& l )
     {
 	s << "<";
@@ -77,12 +81,14 @@ namespace snapper
 	return( s );
     }
 
+
     template<class F, class S> std::ostream& operator<<( std::ostream& s, const std::pair<F,S>& p )
     {
 	s << "[" << p.first << ":" << p.second << "]";
 	return( s );
     }
 
+
     template<class Key, class Value> std::ostream& operator<<( std::ostream& s, const std::map<Key,Value>& m )
     {
 	s << "<";
@@ -97,76 +103,6 @@ namespace snapper
     }
 
 
-    template <typename Type>
-    void logDiff(std::ostream& log, const char* text, const Type& lhs, const Type& rhs)
-    {
-	static_assert(!std::is_enum<Type>::value, "is enum");
-
-	if (lhs != rhs)
-	    log << " " << text << ":" << lhs << "-->" << rhs;
-    }
-
-    template <typename Type>
-    void logDiffHex(std::ostream& log, const char* text, const Type& lhs, const Type& rhs)
-    {
-	static_assert(std::is_integral<Type>::value, "not integral");
-
-	if (lhs != rhs)
-	    log << " " << text << ":" << std::hex << lhs << "-->" << rhs << std::dec;
-    }
-
-    template <typename Type>
-    void logDiffEnum(std::ostream& log, const char* text, const Type& lhs, const Type& rhs)
-    {
-	static_assert(std::is_enum<Type>::value, "not enum");
-
-	if (lhs != rhs)
-	    log << " " << text << ":" << toString(lhs) << "-->" << toString(rhs);
-    }
-
-    inline
-    void logDiff(std::ostream& log, const char* text, bool lhs, bool rhs)
-    {
-	if (lhs != rhs)
-	{
-	    if (rhs)
-		log << " -->" << text;
-	    else
-		log << " " << text << "-->";
-	}
-    }
-
-
-    template<class Type>
-    struct deref_less : public std::binary_function<const Type*, const Type*, bool>
-    {
-	bool operator()(const Type* x, const Type* y) const { return *x < *y; }
-    };
-
-
-    template<class Type>
-    struct deref_equal_to : public std::binary_function<const Type*, const Type*, bool>
-    {
-	bool operator()(const Type* x, const Type* y) const { return *x == *y; }
-    };
-
-
-    template <class Type>
-    void pointerIntoSortedList(list<Type*>& l, Type* e)
-    {
-	l.insert(lower_bound(l.begin(), l.end(), e, deref_less<Type>()), e);
-    }
-
-
-    template <class Type>
-    void clearPointerList(list<Type*>& l)
-    {
-	for (typename list<Type*>::iterator i = l.begin(); i != l.end(); ++i)
-	    delete *i;
-	l.clear();
-    }
-
-
     template <typename ListType, typename Type>
     bool contains(const ListType& l, const Type& value)
     {
@@ -181,27 +117,6 @@ namespace snapper
     }
 
 
-    template<typename Map, typename Key, typename Value>
-    typename Map::iterator mapInsertOrReplace(Map& m, const Key& k, const Value& v)
-    {
-	typename Map::iterator pos = m.lower_bound(k);
-	if (pos != m.end() && !typename Map::key_compare()(k, pos->first))
-	    pos->second = v;
-	else
-	    pos = m.insert(pos, typename Map::value_type(k, v));
-	return pos;
-    }
-
-    template<typename List, typename Value>
-    typename List::const_iterator addIfNotThere(List& l, const Value& v)
-    {
-	typename List::const_iterator pos = find( l.begin(), l.end(), v );
-	if (pos == l.end() )
-	    pos = l.insert(l.end(), v);
-	return pos;
-    }
-
-
     template <class T, unsigned int sz>
     inline unsigned int lengthof(T (&)[sz]) { return sz; }