Fix subtle crash in KviKvsHash destruction when the module that allocated it has been unloaded: the _vtable for KviPointerList<KviPointerHashTableEntry<QString,KviKvsVariant>> must reside inside the kvirc executable

6 files changed, 79 insertions, 17 deletions

diff --git a/cmake/module.rules.txt b/cmake/module.rules.txt
index 10b2b7756..1b0d67853 100644
--- a/cmake/module.rules.txt
+++ b/cmake/module.rules.txt
@@ -9,11 +9,7 @@ endif()
 
 add_library(${kvi_module_name} MODULE ${${kvi_module_name}_SRCS} ${${kvi_module_name}_MOC_SRCS})
 
-if(WIN32)
-	target_link_libraries(${kvi_module_name} ${KVILIB_BINARYNAME} ${KVIRC_BINARYNAME} ${LIBS})
-else()
-	target_link_libraries(${kvi_module_name} ${KVILIB_BINARYNAME} ${LIBS})
-endif()
+target_link_libraries(${kvi_module_name} ${KVILIB_BINARYNAME} ${KVIRC_BINARYNAME} ${LIBS})
 
 if(Qt5Widgets_FOUND)
 	qt5_use_modules(${kvi_module_name} ${qt5_kvirc_modules})
diff --git a/src/kvilib/core/KviPointerList.h b/src/kvilib/core/KviPointerList.h
index e9414cdea..4a9e3aafd 100644
--- a/src/kvilib/core/KviPointerList.h
+++ b/src/kvilib/core/KviPointerList.h
@@ -1233,4 +1233,11 @@ public:
 	template class KVILIB_API KviPointerList<KviCString>;
 #endif
 
+// Provide a default implementation of kvi_compare()
+template<typename T> int kvi_compare(const T * p1,const T * p2)
+{
+	return p1 > p2; // just compare pointers
+}
+
+
 #endif //_KVI_POINTERLIST_H_
diff --git a/src/kvirc/CMakeLists.txt b/src/kvirc/CMakeLists.txt
index 9c45410ac..ced426211 100644
--- a/src/kvirc/CMakeLists.txt
+++ b/src/kvirc/CMakeLists.txt
@@ -267,6 +267,7 @@ if(NOT WANT_DEBUG)
 else()
 	add_executable(${KVIRC_BINARYNAME} ${kvirc_SRCS} ${kvirc_MOC_SRCS})
 endif()
+
 target_link_libraries(${KVIRC_BINARYNAME} ${KVILIB_BINARYNAME} ${LIBS})
 
 if(Qt5Widgets_FOUND)
@@ -280,10 +281,10 @@ if(CMAKE_HOST_SYSTEM MATCHES "FreeBSD")
 	message(STATUS "Patching for FreeBSD...")
 endif()
 
-if(WIN32)
-	# Needed for linking
-	set_target_properties(${KVIRC_BINARYNAME} PROPERTIES ENABLE_EXPORTS ON)
+# This is needed for linking the modules (not actually used by gcc on linux... but it SHOULD be)
+set_target_properties(${KVIRC_BINARYNAME} PROPERTIES ENABLE_EXPORTS ON)
 
+if(WIN32)
 	# We need this defined when mingw will compile moc files
 	add_definitions(-D__KVIRC__)
 endif()
diff --git a/src/kvirc/kvs/KviKvsHash.cpp b/src/kvirc/kvs/KviKvsHash.cpp
index 329ea1683..196b347cd 100644
--- a/src/kvirc/kvs/KviKvsHash.cpp
+++ b/src/kvirc/kvs/KviKvsHash.cpp
@@ -24,6 +24,7 @@
 
 #include "KviKvsHash.h"
 
+
 KviKvsHash::KviKvsHash()
 {
 	m_pDict = new KviPointerHashTable<QString,KviKvsVariant>(17,false);
@@ -47,6 +48,57 @@ KviKvsHash::~KviKvsHash()
 	delete m_pDict;
 }
 
+//
+// Don't inline these short functions as they instantiate a huge template
+// which would be then placed in every single module.
+//
+// There would be also an interesting problem when the modules are unloaded.
+// KviPointerHashTable uses a KviPointerList which has a _vtable.
+// Assume you allocate a KviKvsHash inside a module and the KviPointerList
+// implementation ends up in the module's text segment. Now if you unload the
+// module and then later delete the KviKvsHash (or just cause the destruction
+// of the internal KviPointerList in some way) you'll end up calling the
+// destructor via _vtable. The _vtable will no longer be there and you'll
+// be dead :)
+//
+// It took me a whole day to figure this out.
+//
+
+void KviKvsHash::unset(const QString & szKey)
+{
+	m_pDict->remove(szKey);
+}
+
+void KviKvsHash::set(const QString & szKey, KviKvsVariant * pVal)
+{
+	m_pDict->replace(szKey,pVal);
+}
+
+KviKvsVariant * KviKvsHash::find(const QString & szKey) const
+{
+	return m_pDict->find(szKey);
+}
+
+bool KviKvsHash::isEmpty() const
+{
+	return m_pDict->isEmpty();
+}
+
+void KviKvsHash::clear()
+{
+	m_pDict->clear();
+}
+
+const KviPointerHashTable<QString,KviKvsVariant> * KviKvsHash::dict()
+{
+	return m_pDict;
+}
+
+kvs_uint_t KviKvsHash::size() const
+{
+	return m_pDict->count();
+}
+
 void KviKvsHash::appendAsString(QString & szBuffer) const
 {
 	KviPointerHashTableIterator<QString,KviKvsVariant> it(*m_pDict);
diff --git a/src/kvirc/kvs/KviKvsHash.h b/src/kvirc/kvs/KviKvsHash.h
index c8638d83b..b022e8959 100644
--- a/src/kvirc/kvs/KviKvsHash.h
+++ b/src/kvirc/kvs/KviKvsHash.h
@@ -66,15 +66,14 @@ public:
 	* \brief Destroys the array data
 	*/
 	~KviKvsHash();
-protected:
-	KviPointerHashTable<QString,KviKvsVariant> * m_pDict;
+
 public:
 	/**
 	* \brief Unsets an element from the hash
 	* \param szKey The key of the element to unset
 	* \return void
 	*/
-	void unset(const QString & szKey){ m_pDict->remove(szKey); };
+	void unset(const QString & szKey);
 
 	/**
 	* \brief Sets an element into the hash
@@ -82,14 +81,14 @@ public:
 	* \param pVal The value to set
 	* \return void
 	*/
-	void set(const QString & szKey, KviKvsVariant * pVal){ m_pDict->replace(szKey,pVal); };
+	void set(const QString & szKey, KviKvsVariant * pVal);
 
 	/**
 	* \brief Returns the element associated to the given key
 	* \param szKey The key of the element to retrieve
 	* \return KviKvsVariant *
 	*/
-	KviKvsVariant * find(const QString & szKey) const { return m_pDict->find(szKey); };
+	KviKvsVariant * find(const QString & szKey) const;
 
 	/**
 	* \brief Returns the element associated to the given key
@@ -105,15 +104,18 @@ public:
 	* \brief Returns true if the hash is empty
 	* \return bool
 	*/
-	bool isEmpty() const { return m_pDict->isEmpty(); };
+	bool isEmpty() const;
 
-	void clear(){ m_pDict->clear(); };
+	/**
+	* \brief clear the hash
+	*/
+	void clear();
 
 	/**
 	* \brief Returns the size of the hash
 	* \return kvs_uint_t
 	*/
-	kvs_uint_t size() const { return m_pDict->count(); };
+	kvs_uint_t size() const;
 
 	/**
 	* \brief Appends data to the hash converting it into a string
@@ -126,7 +128,7 @@ public:
 	* \brief Returns the internal dictionary of the hash
 	* \return const KviPointerHashTable<QString,KviKvsVariant> *
 	*/
-	const KviPointerHashTable<QString,KviKvsVariant> * dict(){ return m_pDict; };
+	const KviPointerHashTable<QString,KviKvsVariant> * dict();
 
 	/**
 	* \brief Serializes the hash to a given buffer
@@ -134,6 +136,9 @@ public:
 	* \return void
 	*/
 	void serialize(QString & szResult);
+
+private:
+	KviPointerHashTable<QString,KviKvsVariant> * m_pDict;
 };
 
 #endif // _KVI_KVS_HASH_H_
diff --git a/src/modules/classeditor/ClassEditorWindow.cpp b/src/modules/classeditor/ClassEditorWindow.cpp
index 48151b0f6..d2a7b75b5 100644
--- a/src/modules/classeditor/ClassEditorWindow.cpp
+++ b/src/modules/classeditor/ClassEditorWindow.cpp
@@ -1118,6 +1118,7 @@ void ClassEditorWidget::exportSelectionInSinglesFiles(KviPointerList<ClassEditor
 			m_szDir,
 			__tr2qs_ctx("Choose a Directory - KVIrc","editor"),
 			m_szDir,
+			QString(),
 			false,
 			true,
 			this