Change the ownership semantics of metatype converter functions.

Destroying the instances is a problem because the instances might
have been created in a plugin. In that case, the destructor of
the instance will be implemented in the plugin. As the instance
destruction code is run very late, the plugin might have already
been unloaded by the time an attempt is made to destroy the
instance.

The workaround is to create static instances on the stack.

Task-number: QTBUG-31142

Change-Id: Ic2632c3548a734b742da46d90249916c35705d46
Reviewed-by: Stephen Kelly <stephen.kelly@kdab.com>
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>

2 files changed, 71 insertions, 47 deletions

diff --git a/src/corelib/kernel/qmetatype.cpp b/src/corelib/kernel/qmetatype.cpp
index 5551880e30..5c3f252395 100644
--- a/src/corelib/kernel/qmetatype.cpp
+++ b/src/corelib/kernel/qmetatype.cpp
@@ -428,8 +428,6 @@ public:
     ~QMetaTypeConversionRegistry()
     {
         const QWriteLocker locker(&lock);
-        Q_FOREACH (QtPrivate::AbstractConverterFunction *f, map)
-            f->destroy(f);
         map.clear();
     }
 
@@ -440,27 +438,33 @@ public:
         return map.contains(k);
     }
 
-    bool insertIfNotContains(int from, int to, QtPrivate::AbstractConverterFunction *f)
+    bool insertIfNotContains(int from, int to, const QtPrivate::AbstractConverterFunction *f)
     {
         const Key k(from, to);
         const QWriteLocker locker(&lock);
-        QtPrivate::AbstractConverterFunction* &fun = map[k];
+        const QtPrivate::AbstractConverterFunction* &fun = map[k];
         if (fun != 0)
             return false;
         fun = f;
         return true;
     }
 
-    QtPrivate::AbstractConverterFunction *function(int from, int to) const
+    const QtPrivate::AbstractConverterFunction *function(int from, int to) const
     {
         const Key k(from, to);
         const QReadLocker locker(&lock);
         return map.value(k, 0);
     }
 
+    void remove(int from, int to)
+    {
+        const Key k(from, to);
+        const QWriteLocker locker(&lock);
+        map.remove(k);
+    }
 private:
     mutable QReadWriteLock lock;
-    QHash<Key, QtPrivate::AbstractConverterFunction *> map;
+    QHash<Key, const QtPrivate::AbstractConverterFunction *> map;
 };
 
 namespace
@@ -514,19 +518,27 @@ Q_GLOBAL_STATIC(QMetaTypeConversionRegistry, customTypesConversionRegistry)
     \since 5.2
     \internal
 */
-bool QMetaType::registerConverterFunction(QtPrivate::AbstractConverterFunction *f, int from, int to)
+bool QMetaType::registerConverterFunction(const QtPrivate::AbstractConverterFunction *f, int from, int to)
 {
     if (!customTypesConversionRegistry()->insertIfNotContains(from, to, f)) {
         qWarning("Type conversion already registered from type %s to type %s",
                  QMetaType::typeName(from), QMetaType::typeName(to));
-        if (f)
-            f->destroy(f);
         return false;
     }
     return true;
 }
 
 /*!
+    \internal
+
+    Invoked automatically when a converter function object is destroyed.
+ */
+void QMetaType::unregisterConverterFunction(int from, int to)
+{
+    customTypesConversionRegistry()->remove(from, to);
+}
+
+/*!
     Converts the object at \a from from \a fromTypeId to the preallocated space at \a to
     typed \a toTypeId. Returns true, if the conversion succeeded, otherwise false.
     \since 5.2
diff --git a/src/corelib/kernel/qmetatype.h b/src/corelib/kernel/qmetatype.h
index 0104f0ea56..302c5da63a 100644
--- a/src/corelib/kernel/qmetatype.h
+++ b/src/corelib/kernel/qmetatype.h
@@ -225,20 +225,19 @@ To convertImplicit(const From& from)
 struct AbstractConverterFunction
 {
     typedef bool (*Converter)(const AbstractConverterFunction *, const void *, void*);
-    typedef void (*Destroy)(AbstractConverterFunction *);
-    explicit AbstractConverterFunction(Converter c = 0, Destroy d = 0)
-        : convert(c), destroy(d) {}
+    explicit AbstractConverterFunction(Converter c = 0)
+        : convert(c) {}
     Q_DISABLE_COPY(AbstractConverterFunction)
     Converter convert;
-    Destroy destroy;
 };
 
 template<typename From, typename To>
 struct ConverterMemberFunction : public AbstractConverterFunction
 {
     explicit ConverterMemberFunction(To(From::*function)() const)
-        : AbstractConverterFunction(convert, destroy),
+        : AbstractConverterFunction(convert),
           m_function(function) {}
+    ~ConverterMemberFunction();
     static bool convert(const AbstractConverterFunction *_this, const void *in, void *out)
     {
         const From *f = static_cast<const From *>(in);
@@ -249,11 +248,6 @@ struct ConverterMemberFunction : public AbstractConverterFunction
         return true;
     }
 
-    static void destroy(AbstractConverterFunction *_this)
-    {
-        delete static_cast<ConverterMemberFunction *>(_this);
-    }
-
     To(From::* const m_function)() const;
 };
 
@@ -261,8 +255,9 @@ template<typename From, typename To>
 struct ConverterMemberFunctionOk : public AbstractConverterFunction
 {
     explicit ConverterMemberFunctionOk(To(From::*function)(bool *) const)
-        : AbstractConverterFunction(convert, destroy),
+        : AbstractConverterFunction(convert),
           m_function(function) {}
+    ~ConverterMemberFunctionOk();
     static bool convert(const AbstractConverterFunction *_this, const void *in, void *out)
     {
         const From *f = static_cast<const From *>(in);
@@ -276,11 +271,6 @@ struct ConverterMemberFunctionOk : public AbstractConverterFunction
         return ok;
     }
 
-    static void destroy(AbstractConverterFunction *_this)
-    {
-        delete static_cast<ConverterMemberFunctionOk *>(_this);
-    }
-
     To(From::* const m_function)(bool*) const;
 };
 
@@ -288,8 +278,9 @@ template<typename From, typename To, typename UnaryFunction>
 struct ConverterFunctor : public AbstractConverterFunction
 {
     explicit ConverterFunctor(UnaryFunction function)
-        : AbstractConverterFunction(convert, destroy),
+        : AbstractConverterFunction(convert),
           m_function(function) {}
+    ~ConverterFunctor();
     static bool convert(const AbstractConverterFunction *_this, const void *in, void *out)
     {
         const From *f = static_cast<const From *>(in);
@@ -300,11 +291,6 @@ struct ConverterFunctor : public AbstractConverterFunction
         return true;
     }
 
-    static void destroy(AbstractConverterFunction *_this)
-    {
-        delete static_cast<ConverterFunctor *>(_this);
-    }
-
     UnaryFunction m_function;
 };
 
@@ -468,8 +454,8 @@ public:
 
         const int fromTypeId = qMetaTypeId<From>();
         const int toTypeId = qMetaTypeId<To>();
-        return registerConverterFunction(new QtPrivate::ConverterMemberFunction<From, To>(function),
-                                         fromTypeId, toTypeId);
+        static const QtPrivate::ConverterMemberFunction<From, To> f(function);
+        return registerConverterFunction(&f, fromTypeId, toTypeId);
     }
 
     // member function as in "double QString::toDouble(bool *ok = 0) const"
@@ -481,8 +467,8 @@ public:
 
         const int fromTypeId = qMetaTypeId<From>();
         const int toTypeId = qMetaTypeId<To>();
-        return registerConverterFunction(new QtPrivate::ConverterMemberFunctionOk<From, To>(function),
-                                         fromTypeId, toTypeId);
+        static const QtPrivate::ConverterMemberFunctionOk<From, To> f(function);
+        return registerConverterFunction(&f, fromTypeId, toTypeId);
     }
 
     // functor or function pointer
@@ -494,8 +480,8 @@ public:
 
         const int fromTypeId = qMetaTypeId<From>();
         const int toTypeId = qMetaTypeId<To>();
-        return registerConverterFunction(new QtPrivate::ConverterFunctor<From, To, UnaryFunction>(function),
-                                        fromTypeId, toTypeId);
+        static const QtPrivate::ConverterFunctor<From, To, UnaryFunction> f(function);
+        return registerConverterFunction(&f, fromTypeId, toTypeId);
     }
 #endif
 
@@ -542,11 +528,15 @@ private:
     template<typename T>
     friend bool qRegisterSequentialConverter();
     template<typename, bool> friend struct QtPrivate::ValueTypeIsMetaType;
+    template<typename, typename> friend struct QtPrivate::ConverterMemberFunction;
+    template<typename, typename> friend struct QtPrivate::ConverterMemberFunctionOk;
+    template<typename, typename, typename> friend struct QtPrivate::ConverterFunctor;
 #endif
 #else
 public:
 #endif
-    static bool registerConverterFunction(QtPrivate::AbstractConverterFunction *f, int from, int to);
+    static bool registerConverterFunction(const QtPrivate::AbstractConverterFunction *f, int from, int to);
+    static void unregisterConverterFunction(int from, int to);
 private:
 
     Creator m_creator;
@@ -567,6 +557,26 @@ private:
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QMetaType::TypeFlags)
 
+namespace QtPrivate {
+
+template<typename From, typename To>
+ConverterMemberFunction<From, To>::~ConverterMemberFunction()
+{
+    QMetaType::unregisterConverterFunction(qMetaTypeId<From>(), qMetaTypeId<To>());
+}
+template<typename From, typename To>
+ConverterMemberFunctionOk<From, To>::~ConverterMemberFunctionOk()
+{
+    QMetaType::unregisterConverterFunction(qMetaTypeId<From>(), qMetaTypeId<To>());
+}
+template<typename From, typename To, typename UnaryFunction>
+ConverterFunctor<From, To, UnaryFunction>::~ConverterFunctor()
+{
+    QMetaType::unregisterConverterFunction(qMetaTypeId<From>(), qMetaTypeId<To>());
+}
+
+}
+
 namespace QtMetaTypePrivate {
 template <typename T, bool Accepted = true>
 struct QMetaTypeFunctionHelper {
@@ -857,6 +867,8 @@ public:
 template<typename From>
 struct QSequentialIterableConvertFunctor
 {
+    QSequentialIterableConvertFunctor() {}
+
     QSequentialIterableImpl operator()(const From &f) const
     {
         return QSequentialIterableImpl(&f);
@@ -991,12 +1003,11 @@ namespace QtPrivate
         {
             const int toId = qMetaTypeId<QtMetaTypePrivate::QSequentialIterableImpl>();
             if (!QMetaType::hasRegisteredConverterFunction(id, toId)) {
-                QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> f;
-                return QMetaType::registerConverterFunction(
-                    new QtPrivate::ConverterFunctor<T,
+                static const QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> o;
+                static const QtPrivate::ConverterFunctor<T,
                                                   QtMetaTypePrivate::QSequentialIterableImpl,
-                                                  QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> >(f),
-                  id, toId);
+                    QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> > f(o);
+                return QMetaType::registerConverterFunction(&f, id, toId);
             }
             return true;
         }
@@ -1505,11 +1516,12 @@ bool qRegisterSequentialConverter()
     if (QMetaType::hasRegisteredConverterFunction(id, toId))
         return true;
 
-    QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> f;
-    return QMetaType::registerConverterFunction(
-        new QtPrivate::ConverterFunctor<T,
+    static const QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> o;
+    static const QtPrivate::ConverterFunctor<T,
             QtMetaTypePrivate::QSequentialIterableImpl,
-            QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> >(f),
+            QtMetaTypePrivate::QSequentialIterableConvertFunctor<T> > f(o);
+    return QMetaType::registerConverterFunction(
+        &f,
           id, toId);
 }