Browse Source

Fix memory management in QQuickGrpcSubscription

- Store QGrpcSubscription in QPointer
- Set return value CppOwnership
- Make correct delete of return value
- Add docygen description for QML QtGrpc module
- Rename QtProtobufQuick module to QML QtProtobuf

Fixes: #69
Alexey Edelev 4 years ago
parent
commit
6ed72a7b8c

+ 2 - 1
src/generator/main.cpp

@@ -42,7 +42,8 @@
  *  - \ref QtProtobuf
  *  - \ref QtGrpc
  *  - \ref QtProtobufWellKnownTypes
- *  - \ref QtProtobufQuick
+ *  - \ref QtProtobufQML
+ *  - \ref QtGrpcQML
  *
  * \subsection gettingstarted Getting Started with QtProtobuf
  *

+ 15 - 2
src/grpc/qgrpcstatus.h

@@ -36,6 +36,19 @@ namespace QtProtobuf {
 
 class QGrpcStatusPrivate;
 
+/*!
+ * \ingroup QtGrpcQML
+ * \class GrpcStatus
+ * \brief GrpcStatus is QML representation of QtProtobuf::QGrpcStatus.
+ *
+ * \subsection Properties
+ * \subsubsection code QtProtobuf::QGrpcStatus::StatusCode code
+ * \details QtProtobuf::QGrpcStatus::StatusCode received for prior gRPC call.
+ *
+ * \subsubsection message QString message
+ * \details Status message received for prior gRPC call.
+ */
+
 /*!
  * \ingroup QtGrpc
  * \brief The QGrpcStatus class contains information about last gRPC operation. In case of error in call/subscription
@@ -79,12 +92,12 @@ public:
     ~QGrpcStatus();
 
     /*!
-     * \brief code getter for QGrpcStatus::StatusCode stored in QGrpcStatus
+     * \brief code getter for QGrpcStatus::StatusCode received for prior gRPC call.
      */
     StatusCode code() const;
 
     /*!
-     * \brief message getter for status message stored in QGrpcStatus
+     * \brief message getter for status message received for prior gRPC call.
      */
     QString message() const;
 

+ 18 - 5
src/grpc/quick/qquickgrpcsubscription.cpp

@@ -28,23 +28,30 @@
 
 #include <QGrpcSubscription>
 #include <QJSEngine>
+#include <QQmlEngine>
 
 using namespace QtProtobuf;
 
 QQuickGrpcSubscription::QQuickGrpcSubscription(QObject *parent) : QObject(parent)
   , m_enabled(false)
-  , m_subscription(nullptr)
   , m_returnValue(nullptr)
 {
+}
 
+QQuickGrpcSubscription::~QQuickGrpcSubscription()
+{
+    delete m_returnValue;
 }
 
+
 void QQuickGrpcSubscription::updateSubscription()
 {
-    if (m_subscription != nullptr) {
+    if (!m_subscription.isNull()) {
         m_subscription->cancel();
         m_subscription = nullptr;
+    }
 
+    if (m_returnValue != nullptr) {
         m_returnValue->deleteLater(); //TODO: probably need to take care about return value cleanup other way. It's just reminder about weak memory management.
         m_returnValue = nullptr;
     }
@@ -128,6 +135,7 @@ bool QQuickGrpcSubscription::subscribe()
     }
 
     m_returnValue = reinterpret_cast<QObject*>(returnMetaType.create());
+    qmlEngine(this)->setObjectOwnership(m_returnValue, QQmlEngine::CppOwnership);
 
     if (m_returnValue == nullptr) {
         errorString = "Unable to allocate return value. Unknown metatype system error";
@@ -136,22 +144,27 @@ bool QQuickGrpcSubscription::subscribe()
         return false;
     }
 
+    QGrpcSubscription *subscription = nullptr;
     bool ok = method.invoke(m_client, Qt::DirectConnection,
-                                      QGenericReturnArgument("QtProtobuf::QGrpcSubscription*", static_cast<void *>(&m_subscription)),
+                                      QGenericReturnArgument("QtProtobuf::QGrpcSubscription*", static_cast<void *>(&subscription)),
                                       QGenericArgument(method.parameterTypes().at(0).data(), static_cast<const void *>(&argument)),
                                       QGenericArgument(method.parameterTypes().at(1).data(), static_cast<const void *>(&m_returnValue)));
-    if (!ok) {
+    if (!ok || subscription == nullptr) {
         errorString = QString("Unable to call ") + m_method + "invalidate subscription.";
         qProtoWarning() << errorString;
         error({QGrpcStatus::Unknown, errorString});
         return false;
     }
 
+    m_subscription = subscription;
+
     connect(m_subscription, &QGrpcSubscription::updated, this, [this](){
         updated(qjsEngine(this)->toScriptValue(m_returnValue));
     });
 
-    connect(m_subscription, &QGrpcSubscription::error, this, &QQuickGrpcSubscription::error);
+    connect(m_subscription, &QGrpcSubscription::error, this, &QQuickGrpcSubscription::error);//TODO: Probably it's good idea to disable subscription here
+
+    connect(m_subscription, &QGrpcSubscription::finished, this, [this](){ setEnabled(false); });
 
     return true;
 }

+ 57 - 2
src/grpc/quick/qquickgrpcsubscription_p.h

@@ -29,12 +29,67 @@
 #include <QAbstractGrpcClient>
 #include <QGrpcStatus>
 
+/*!
+ * \ingroup QtGrpcQML
+ * \class GrpcSubscription
+ * \brief GrpcSubscription provides access to gRPC subscriptions from QML.
+ *
+ * \details GrpcSubscription might be used from QML code to receive updates for gRPC server or bidirectional streaming methods.
+ * Follwing properties should be provided and can not be empty, to subscribe streaming method:
+ * - client
+ * - method
+ * - argument
+ * Changing any of this properties cause subscription cancelation and re-initialization.
+ *
+ * \subsection Properties
+ * \subsubsection client QtProtobuf::QAbstractGrpcClient *client
+ * \details Client used for subscription.
+ *
+ * \subsubsection enabled bool enabled
+ * \details Controls subscription status. If subscription is active, switch this flag to 'false'
+ * to cancel subscription. Switching to 'false' keeps all fields ready to restore subscription.
+ *
+ * \subsubsection method QString method
+ * \details The name of streaming method that will be used for subscription.
+ *
+ * \subsubsection argument QObject *argument
+ * \details Pointer to argument that will be used for subscription.
+ *
+ * \subsection Signals
+ * \subsubsection updated updated(ReturnType value)
+ * \details The signal notifies about received update for subscription. It provides "return" value ready for use in QML.
+ * \code
+ * GrpcSubscription {
+ *     ...
+ *     onUpdated: {
+ *         //Use value passed as argument to binding
+ *         ...
+ *     }
+ * }
+ * \endcode
+ *
+ * \subsubsection error error(QtProtobuf::QGrpcStatus status)
+ * \details The signal notifies about error occured for subscription. Provides GrpcStatus as argument to assigned handler.
+ * \note Some errors at subscription initialization phase disable GrpcSubscription
+ *
+ * \code
+ * GrpcSubscription {
+ *     id: mySubscription
+ *     ...
+ *     onError: {
+ *         console.log("Subscription error: " + status.code + " " + status.message)
+ *     }
+ * }
+ * \endcode
+ */
+
 class QJSValue;
 
 namespace QtProtobuf {
 
 class QGrpcSubscription;
 
+//! \private
 class QQuickGrpcSubscription : public QObject
 {
     Q_OBJECT
@@ -45,7 +100,7 @@ class QQuickGrpcSubscription : public QObject
 
 public:
     QQuickGrpcSubscription(QObject *parent = nullptr);
-    ~QQuickGrpcSubscription() = default;
+    ~QQuickGrpcSubscription();
 
     QAbstractGrpcClient *client() const {
         return m_client;
@@ -119,7 +174,7 @@ private:
     bool m_enabled;
     QString m_method;
     QPointer<QObject> m_argument;
-    QGrpcSubscription *m_subscription;
+    QPointer<QGrpcSubscription> m_subscription;
     QObject *m_returnValue;
 };
 

+ 5 - 0
src/grpc/quick/qtgrpcquickplugin.h

@@ -28,6 +28,11 @@
 #include <QQmlExtensionPlugin>
 #include "qtgrpcquick_global.h"
 
+/*!
+ * \defgroup QtGrpcQML QML QtGrpc
+ * \brief QML bindings for QtGrpc
+ */
+
 /*!
  * \private
  * \brief The QtGrpcQuickPlugin class

+ 2 - 1
src/protobuf/quick/qtprotobufquickplugin.h

@@ -29,7 +29,8 @@
 #include "qtprotobufquick_global.h"
 
 /*!
- * \defgroup QtProtobufQuick
+ * \defgroup QtProtobufQML QML QtProtobuf
+ * \brief QML bindings for QtGrpc
  * \details
  * QtProtobuf allows to use generated classes in QML. All types are registred automatically when QtProtobuf::qRegisterProtobufTypes() called.
  * To make generated classes and QtProtobuf types visible in QML you need to import QtProtobuf QML plugin and your protobuf package