Fixed double delete in CAPITimer and add documentation for platform networking CAPI integration

CHANGELOG.md

@@ -16,11 +16,13 @@
 * If a user was logged out while an access token refresh was in progress, the refresh completing would mark the user as logged in again and the user would be in an inconsistent state ([PR #6837](https://github.com/realm/realm-core/pull/6837), since v10.0.0).
 * If querying over a geospatial dataset that had some objects with a type property set to something other than 'Point' (case insensitive) an exception would have been thrown. Instead of disrupting the query, those objects are now just ignored. ([PR 6989](https://github.com/realm/realm-core/issues/6989), since the introduction of geospatial)
 * The Swift package failed to link required libraries when building for macCatalyst.
+* Fixed issue with double delete when using the CAPI for timers in platform networking ([#6993](https://github.com/realm/realm-core/issues/6993), since v13.3.0).
 
 ### Breaking changes
 * SyncUser::provider_type() and realm_user_get_auth_provider() have been removed. Users don't have provider types; identities do. `SyncUser::is_anonymous()` is a more correct version of checking if the provider type is anonymous ([PR #6837](https://github.com/realm/realm-core/pull/6837)).
 * SyncUser no longer has a `local_identity()`. `identity()` has been guaranteed to be unique per App ever since v10 ([PR #6837](https://github.com/realm/realm-core/pull/6837)).
 * SyncUser no longer overrides operator==. Pointer equality should be used to compare sync users ([PR #6837](https://github.com/realm/realm-core/pull/6837)).
+* Platform Networking CAPI has been updated to provide 3 different functions (instead of 1) for executing callback handlers depending on purpose ([PR #6994](https://github.com/realm/realm-core/pull/6994)).
 
 ### Compatibility
 * Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5.

src/realm.h

@@ -4061,6 +4061,25 @@ RLM_API bool realm_mongo_collection_find_one_and_delete(realm_mongodb_collection
                                                         realm_userdata_t data, realm_free_userdata_func_t delete_data,
                                                         realm_mongodb_callback_t callback);
 
+/**
+ * Creates a new sync socket instance for the Sync Client that handles the operations for a custom
+ * websocket and event loop implementation.
+ * @param userdata CAPI implementation specific pointer containing custom context data that is provided to
+ *                 each of the provided function.
+ * @param userdata_free function that will be called when the sync socket is destroyed to delete userdata.
+ * @param post_func function that will be called to post a callback handler onto the event loop - use the
+ *                  realm_sync_socket_post_complete() function when the callback handler is scheduled to run.
+ * @param create_timer_func function that will be called to create a new timer resource with the callback
+ *                          handler that will be run when the timer expires or an erorr occurs - use the
+ *                          realm_sync_socket_timer_canceled() function if the timer is canceled or the
+ *                          realm_sync_socket_timer_complete() function if the timer expires or an error occurs.
+ * @param cancel_timer_func function that will be called when the timer has been canceled by the sync client.
+ * @param free_timer_func function that will be called when the timer resource has been destroyed by the sync client.
+ * @param websocket_connect_func function that will be called when the sync client creates a websocket.
+ * @param websocket_write_func function that will be called when the sync client sends data over the websocket.
+ * @param websocket_free_func function that will be called when the sync client closes the websocket conneciton.
+ * @return a realm_sync_socket_t pointer suitable for passing to realm_sync_client_config_set_sync_socket()
+ */
 RLM_API realm_sync_socket_t* realm_sync_socket_new(
     realm_userdata_t userdata, realm_free_userdata_func_t userdata_free, realm_sync_socket_post_func_t post_func,
     realm_sync_socket_create_timer_func_t create_timer_func,
@@ -4069,17 +4088,79 @@ RLM_API realm_sync_socket_t* realm_sync_socket_new(
     realm_sync_socket_websocket_async_write_func_t websocket_write_func,
     realm_sync_socket_websocket_free_func_t websocket_free_func);
 
-RLM_API void realm_sync_socket_callback_complete(realm_sync_socket_callback_t* realm_callback, realm_errno_e status,
-                                                 const char* reason);
+/**
+ * This function is to be called by the CAPI Implementer when the timer expires or an error occurs.
+ * @param timer_handler the timer callback handler that was provided when the timer was created.
+ * @param code the error code for the error that occurred or RLM_ERR_NONE if the timer expired normally.
+ * @param reason a string describing details about the error that occurred or empty string if no error.
+ * NOTE: This function must be called in the same "thread" as the timer canceled and SyncSocket::Post operations.
+ */
+RLM_API void realm_sync_socket_timer_complete(realm_sync_socket_callback_t* timer_handler, realm_errno_e code,
+                                              const char* reason);
+
+/**
+ * This function is to be called by the CAPI implementer when the timer is canceled by the Sync Client, either
+ * directly or as a result of the CAPITimer object being destroyed.
+ * @param timer_handler the timer callback handler that was provided when the timer was created.
+ * NOTE: This function must be called in the same "thread" as the timer complete and SyncSocket::Post operations.
+ */
+RLM_API void realm_sync_socket_timer_canceled(realm_sync_socket_callback_t* timer_handler);
+
+/**
+ * To be called to execute the callback function provided to the post_func when the event loop executes
+ * that post'ed operation. The realm_callback resource will automatically be destroyed during this
+ * operation.
+ * @param post_handler the post callback handler that was originally provided to the post_func when
+ */
+RLM_API void realm_sync_socket_post_complete(realm_sync_socket_callback_t* post_handler, realm_errno_e status,
+                                             const char* reason);
 
+
+/**
+ * To be called when the websocket successfully connects to the server.
+ * @param realm_websocket_observer the websocket observer object that was provided to the websocket_connect_func
+ * @param protocol the value of the Sec-WebSocket-Protocol header in the connect response from the server.
+ * NOTE: This function should not be called after the websocket_free_func has been called to release
+ * the websocket resources.
+ */
 RLM_API void realm_sync_socket_websocket_connected(realm_websocket_observer_t* realm_websocket_observer,
                                                    const char* protocol);
 
+/**
+ * To be called when an error occurs - the actual error value with be provided when the websocket_closed
+ * function is called. This function informs that the socket object is in an error state and no further
+ * TX operations should be performed.
+ * @param realm_websocket_observer the websocket observer object that was provided to the websocket_connect_func
+ * NOTE: This function should not be called after the websocket_free_func has been called to release
+ * the websocket resources.
+ */
 RLM_API void realm_sync_socket_websocket_error(realm_websocket_observer_t* realm_websocket_observer);
 
+/**
+ * To be called to provide the received data to the Sync Client when a write operation has completed.
+ * The data buffer can be safely discarded after this function has completed.
+ * @param realm_websocket_observer the websocket observer object that was provided to the websocket_connect_func
+ * @param data a pointer to the buffer that contains the data received over the websocket
+ * @param data_size the number of bytes in the data buffer
+ * NOTE: This function should not be called after the websocket_free_func has been called to release
+ * the websocket resources.
+ */
 RLM_API void realm_sync_socket_websocket_message(realm_websocket_observer_t* realm_websocket_observer,
                                                  const char* data, size_t data_size);
 
+/**
+ * To be called when the websocket has been closed, either due to an error or a normal close operation.
+ * @param realm_websocket_observer the websocket observer object that was provided to the websocket_connect_func
+ * @param was_clean boolean value that indicates whether this is a normal close situation (true), the
+ *                  error was provided by the server via a close message (true), or if the error was
+ *                  generated by the local websocket as a result of some other error (false) (e.g. host
+ *                  unreachable, etc.)
+ * @param status the websocket error code that describes why the websocket was closed, or
+ *               RLM_ERR_WEBSOCKET_OK if the socket was closed normally.
+ * @param reason a string describing details about the error that occurred or empty string if no error.
+ * NOTE: This function should not be called after the websocket_free_func has been called to release
+ * the websocket resources.
+ */
 RLM_API void realm_sync_socket_websocket_closed(realm_websocket_observer_t* realm_websocket_observer, bool was_clean,
                                                 realm_web_socket_errno_e status, const char* reason);
 

src/realm/object-store/c_api/socket_provider.cpp

@@ -7,6 +7,8 @@
 namespace realm::c_api {
 namespace {
 
+// THis class represents the timer resource that is returned to the sync client from the
+// CAPI implementation details for canceling and deleting the timer resources.
 struct CAPITimer : sync::SyncSocketProvider::Timer {
 public:
     CAPITimer(realm_userdata_t userdata, int64_t delay_ms, realm_sync_socket_callback_t* handler,
@@ -29,22 +31,48 @@ struct CAPITimer : sync::SyncSocketProvider::Timer {
         realm_release(m_handler);
     }
 
-    /// Cancel the timer immediately.
+    // Cancel the timer immediately - the CAPI implementation will need to call the
+    // realm_sync_socket_timer_canceled function to notify the sync client that the
+    // timer has been canceled and must be called in the same execution thread as
+    // the timer complete.
     void cancel() override
     {
         m_timer_cancel(m_userdata, m_timer);
     }
 
 private:
+    // A pointer to the CAPI implementation's timer instance. This is provided by the
+    // CAPI implementation when the create_timer_func function is called.
     realm_sync_socket_timer_t m_timer = nullptr;
 
-    realm_userdata_t m_userdata = nullptr;
+    // A wrapped reference to the callback function to be called when the timer completes,
+    // is canceled or an error occurs. This is provided by the Sync Client
     realm_sync_socket_callback_t* m_handler = nullptr;
+
+    // These values were originally provided to the socket_provider instance by the CAPI
+    // implementation when it was created
+    realm_userdata_t m_userdata = nullptr;
     realm_sync_socket_create_timer_func_t m_timer_create = nullptr;
     realm_sync_socket_timer_canceled_func_t m_timer_cancel = nullptr;
     realm_sync_socket_timer_free_func_t m_timer_free = nullptr;
 };
 
+RLM_API void realm_sync_socket_timer_complete(realm_sync_socket_callback* timer_handler, realm_errno_e code,
+                                              const char* reason)
+{
+    auto complete_status =
+        code == realm_errno_e::RLM_ERR_NONE ? Status::OK() : Status{static_cast<ErrorCodes::Error>(code), reason};
+    (*(timer_handler->get()))(complete_status);
+}
+
+RLM_API void realm_sync_socket_timer_canceled(realm_sync_socket_callback* timer_handler)
+{
+    realm_sync_socket_timer_complete(timer_handler, RLM_ERR_OPERATION_ABORTED, "Operation canceled");
+}
+
+// This class represents a websocket instance provided by the CAPI implememtation for sending
+// and receiving data and connection state from the websocket. This class is used directly by
+// the sync client.
 struct CAPIWebSocket : sync::WebSocketInterface {
 public:
     CAPIWebSocket(realm_userdata_t userdata, realm_sync_socket_connect_func_t websocket_connect_func,
@@ -89,15 +117,24 @@ struct CAPIWebSocket : sync::WebSocketInterface {
     }
 
 private:
+    // A pointer to the CAPI implementation's websocket instance. This is provided by
+    // the m_websocket_connect() function when this websocket instance is created.
     realm_sync_socket_websocket_t m_socket = nullptr;
+
+    // A wrapped reference to the websocket observer in the sync client that receives the
+    // websocket status callbacks. This is provided by the Sync Client.
     realm_websocket_observer_t* m_observer = nullptr;
-    realm_userdata_t m_userdata = nullptr;
 
+    // These values were originally provided to the socket_provider instance by the CAPI
+    // implementation when it was created.
+    realm_userdata_t m_userdata = nullptr;
     realm_sync_socket_connect_func_t m_websocket_connect = nullptr;
     realm_sync_socket_websocket_async_write_func_t m_websocket_async_write = nullptr;
     realm_sync_socket_websocket_free_func_t m_websocket_free = nullptr;
 };
 
+// Represents the websocket observer in the sync client that receives websocket status
+// callbacks and passes them along to the WebSocketObserver object.
 struct CAPIWebSocketObserver : sync::WebSocketObserver {
 public:
     CAPIWebSocketObserver(std::unique_ptr<sync::WebSocketObserver> observer)
@@ -131,6 +168,10 @@ struct CAPIWebSocketObserver : sync::WebSocketObserver {
     std::unique_ptr<sync::WebSocketObserver> m_observer;
 };
 
+// This is the primary resource for providing event loop, timer and websocket
+// resources and synchronization for the Sync Client. The CAPI implementation
+// needs to implement the "funct_t" functions provided to this class for connecting
+// the implementation to the operations called by the Sync Client.
 struct CAPISyncSocketProvider : sync::SyncSocketProvider {
     realm_userdata_t m_userdata = nullptr;
     realm_free_userdata_func_t m_free = nullptr;
@@ -169,6 +210,9 @@ struct CAPISyncSocketProvider : sync::SyncSocketProvider {
         m_free(m_userdata);
     }
 
+    // Create a websocket object that will be returned to the Sync Client, which is expected to
+    // begin connecting to the endpoint as soon as the object is created. The state and any data
+    // received is passed to the socket observer via the helper functions defined below this class.
     std::unique_ptr<sync::WebSocketInterface> connect(std::unique_ptr<sync::WebSocketObserver> observer,
                                                       sync::WebSocketEndpoint&& endpoint) final
     {
@@ -218,8 +262,8 @@ RLM_API realm_sync_socket_t* realm_sync_socket_new(
     });
 }
 
-RLM_API void realm_sync_socket_callback_complete(realm_sync_socket_callback* realm_callback, realm_errno_e code,
-                                                 const char* reason)
+RLM_API void realm_sync_socket_post_complete(realm_sync_socket_callback* realm_callback, realm_errno_e code,
+                                             const char* reason)
 {
     auto complete_status =
         code == realm_errno_e::RLM_ERR_NONE ? Status::OK() : Status{static_cast<ErrorCodes::Error>(code), reason};

test/object-store/c_api/c_api.cpp

@@ -6093,8 +6093,7 @@ TEST_CASE("C API app: websocket provider", "[sync][app][c_api][baas]") {
         auto test_data = static_cast<TestData*>(userdata);
         REQUIRE(test_data);
         auto cb = [callback_copy = callback](Status s) {
-            realm_sync_socket_callback_complete(callback_copy, static_cast<realm_errno_e>(s.code()),
-                                                s.reason().c_str());
+            realm_sync_socket_post_complete(callback_copy, static_cast<realm_errno_e>(s.code()), s.reason().c_str());
         };
         test_data->socket_provider->post(std::move(cb));
     };