Close without destroy

CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.15)
 cmake_policy(SET CMP0091 NEW)
 cmake_policy(SET CMP0042 NEW)
 
-project(node_datachannel VERSION 0.6.0)
+project(node_datachannel VERSION 0.7.0)
 
 # -Dnapi_build_version=8
 add_definitions(-DNAPI_VERSION=8)

jest.config.cjs

@@ -0,0 +1,7 @@
+/** @type {import('jest').Config} */
+const config = {
+    verbose: true,
+    testPathIgnorePatterns: ['<rootDir>/node_modules/', 'multiple-run.test'],
+};
+
+module.exports = config;

lib/index.d.ts

@@ -275,7 +275,6 @@ export class WebSocketServer {
 export class PeerConnection {
     constructor(peerName: string, config: RtcConfig);
     close(): void;
-    destroy(): void;
     setLocalDescription(type?: DescriptionType): void;
     setRemoteDescription(sdp: string, type: DescriptionType): void;
     localDescription(): { type: string; sdp: string } | null;

package.json

@@ -1,6 +1,6 @@
 {
     "name": "node-datachannel",
-    "version": "0.6.0",
+    "version": "0.7.0-dev",
     "description": "libdatachannel node bindings",
     "type": "module",
     "exports": {

polyfill/RTCPeerConnection.js

@@ -222,7 +222,6 @@ export default class _RTCPeerConnection extends EventTarget {
         });
 
         this.#peerConnection.close();
-        this.#peerConnection.destroy();
     }
 
     createAnswer() {

src/data-channel-wrapper.cpp

@@ -13,6 +13,14 @@ void DataChannelWrapper::CloseAll()
         inst->doClose();
 }
 
+void DataChannelWrapper::CleanupAll()
+{
+    PLOG_DEBUG << "CleanupAll() called";
+    auto copy(instances);
+    for (auto inst : copy)
+        inst->doCleanup();
+}
+
 Napi::Object DataChannelWrapper::Init(Napi::Env env, Napi::Object exports)
 {
     Napi::HandleScope scope(env);
@@ -78,11 +86,15 @@ void DataChannelWrapper::doClose()
     }
 
     mOnOpenCallback.reset();
-    mOnClosedCallback.reset();
     mOnErrorCallback.reset();
     mOnBufferedAmountLowCallback.reset();
     mOnMessageCallback.reset();
+}
 
+void DataChannelWrapper::doCleanup()
+{
+    PLOG_DEBUG << "doCleanup() called";
+    mOnClosedCallback.reset();
     instances.erase(this);
 }
 
@@ -350,6 +362,8 @@ void DataChannelWrapper::onClosed(const Napi::CallbackInfo &info)
                 // This will run in main thread and needs to construct the
                 // arguments for the call
                 args = {};
+            },[this]{
+                doCleanup();
             }); });
 }
 

src/data-channel-wrapper.h

@@ -42,10 +42,14 @@ class DataChannelWrapper : public Napi::ObjectWrap<DataChannelWrapper>
   // Close all existing DataChannels
   static void CloseAll();
 
+  // Reset all Callbacks for existing DataChannels
+  static void CleanupAll();
+
 private:
-  static std::unordered_set<DataChannelWrapper*> instances;
+  static std::unordered_set<DataChannelWrapper *> instances;
 
   void doClose();
+  void doCleanup();
 
   std::string mLabel;
   std::shared_ptr<rtc::DataChannel> mDataChannelPtr = nullptr;

src/media-track-wrapper.cpp

@@ -2,6 +2,8 @@
 #include "media-direction.h"
 #include "media-rtcpreceivingsession-wrapper.h"
 
+#include "plog/Log.h"
+
 Napi::FunctionReference TrackWrapper::constructor;
 std::unordered_set<TrackWrapper *> TrackWrapper::instances;
 
@@ -12,6 +14,14 @@ void TrackWrapper::CloseAll()
         inst->doClose();
 }
 
+void TrackWrapper::CleanupAll()
+{
+    PLOG_DEBUG << "CleanupAll() called";
+    auto copy(instances);
+    for (auto inst : copy)
+        inst->doCleanup();
+}
+
 Napi::Object TrackWrapper::Init(Napi::Env env, Napi::Object exports)
 {
     Napi::HandleScope scope(env);
@@ -73,10 +83,14 @@ void TrackWrapper::doClose()
     }
 
     mOnOpenCallback.reset();
-    mOnClosedCallback.reset();
     mOnErrorCallback.reset();
     mOnMessageCallback.reset();
+}
 
+void TrackWrapper::doCleanup()
+{
+    PLOG_DEBUG << "doCleanup() called";
+    mOnClosedCallback.reset();
     instances.erase(this);
 }
 
@@ -276,7 +290,8 @@ void TrackWrapper::onOpen(const Napi::CallbackInfo &info)
     // Callback
     mOnOpenCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
 
-    mTrackPtr->onOpen([&]() {
+    mTrackPtr->onOpen([&]()
+                      {
         if (mOnOpenCallback)
             mOnOpenCallback->call([this](Napi::Env env, std::vector<napi_value> &args) {
                 // Check the track is not closed
@@ -309,14 +324,17 @@ void TrackWrapper::onClosed(const Napi::CallbackInfo &info)
     // Callback
     mOnClosedCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
 
-    mTrackPtr->onClosed([&]() {
+    mTrackPtr->onClosed([&]()
+                        {
         if (mOnClosedCallback)
             mOnClosedCallback->call([this](Napi::Env env, std::vector<napi_value> &args) {
                 // Do not check if the data channel has been closed here
 
                 // This will run in main thread and needs to construct the
                 // arguments for the call
                 args = {};
+            },[this]{
+                doCleanup();
             }); });
 }
 
@@ -340,7 +358,8 @@ void TrackWrapper::onError(const Napi::CallbackInfo &info)
     // Callback
     mOnErrorCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
 
-    mTrackPtr->onError([&](const std::string &error) {
+    mTrackPtr->onError([&](const std::string &error)
+                       {
         if (mOnErrorCallback)
             mOnErrorCallback->call([this, error](Napi::Env env, std::vector<napi_value> &args) {
                 // Check the track is not closed
@@ -373,7 +392,8 @@ void TrackWrapper::onMessage(const Napi::CallbackInfo &info)
     // Callback
     mOnMessageCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
 
-    mTrackPtr->onMessage([&](std::variant<rtc::binary, std::string> message) {
+    mTrackPtr->onMessage([&](std::variant<rtc::binary, std::string> message)
+                         {
         if (mOnMessageCallback)
             mOnMessageCallback->call([this, message = std::move(message)](Napi::Env env, std::vector<napi_value> &args) {
                 // Check the track is not closed

src/media-track-wrapper.h

@@ -39,21 +39,25 @@ class TrackWrapper : public Napi::ObjectWrap<TrackWrapper>
   void onError(const Napi::CallbackInfo &info);
   void onMessage(const Napi::CallbackInfo &info);
 
-  // Close all existing track
+  // Close all existing tracks
   static void CloseAll();
 
+  // Reset all Callbacks for existing tracks
+  static void CleanupAll();
+
 private:
   static std::unordered_set<TrackWrapper *> instances;
 
   void doClose();
+  void doCleanup();
 
   std::shared_ptr<rtc::Track> mTrackPtr = nullptr;
 
-    // Callback Ptrs
+  // Callback Ptrs
   std::unique_ptr<ThreadSafeCallback> mOnOpenCallback = nullptr;
   std::unique_ptr<ThreadSafeCallback> mOnClosedCallback = nullptr;
   std::unique_ptr<ThreadSafeCallback> mOnErrorCallback = nullptr;
   std::unique_ptr<ThreadSafeCallback> mOnMessageCallback = nullptr;
 };
 
-#endif // MEDIA_TRACK_WRAPPER_H
+#endif // MEDIA_TRACK_WRAPPER_H

src/peer-connection-wrapper.cpp

@@ -20,12 +20,12 @@ void PeerConnectionWrapper::CloseAll()
         inst->doClose();
 }
 
-void PeerConnectionWrapper::ResetCallbacksAll()
+void PeerConnectionWrapper::CleanupAll()
 {
-    PLOG_DEBUG << "ResetCallbacksAll() called";
+    PLOG_DEBUG << "CleanupAll() called";
     auto copy(instances);
     for (auto inst : copy)
-        inst->doResetCallbacks();
+        inst->doCleanup();
 }
 
 Napi::Object PeerConnectionWrapper::Init(Napi::Env env, Napi::Object exports)
@@ -37,7 +37,6 @@ Napi::Object PeerConnectionWrapper::Init(Napi::Env env, Napi::Object exports)
         "PeerConnection",
         {
             InstanceMethod("close", &PeerConnectionWrapper::close),
-            InstanceMethod("destroy", &PeerConnectionWrapper::destroy),
             InstanceMethod("setLocalDescription", &PeerConnectionWrapper::setLocalDescription),
             InstanceMethod("setRemoteDescription", &PeerConnectionWrapper::setRemoteDescription),
             InstanceMethod("localDescription", &PeerConnectionWrapper::localDescription),
@@ -253,7 +252,8 @@ PeerConnectionWrapper::PeerConnectionWrapper(const Napi::CallbackInfo &info) : N
 PeerConnectionWrapper::~PeerConnectionWrapper()
 {
     PLOG_DEBUG << "Destructor called";
-    doDestroy();
+    doCleanup();
+    doClose();
 }
 
 void PeerConnectionWrapper::doClose()
@@ -273,40 +273,27 @@ void PeerConnectionWrapper::doClose()
             return;
         }
     }
-}
-
-void PeerConnectionWrapper::close(const Napi::CallbackInfo &info)
-{
-    PLOG_DEBUG << "close() called";
-    doClose();
-}
-
-void PeerConnectionWrapper::doDestroy()
-{
-    PLOG_DEBUG << "doDestroy() called";
-    doClose();
-    doResetCallbacks();
-}
 
-void PeerConnectionWrapper::doResetCallbacks()
-{
-    PLOG_DEBUG << "doResetCallbacks() called";
     mOnLocalDescriptionCallback.reset();
     mOnLocalCandidateCallback.reset();
-    mOnStateChangeCallback.reset();
     mOnIceStateChangeCallback.reset();
     mOnSignalingStateChangeCallback.reset();
     mOnGatheringStateChangeCallback.reset();
     mOnDataChannelCallback.reset();
     mOnTrackCallback.reset();
+}
 
-    instances.erase(this);
+void PeerConnectionWrapper::close(const Napi::CallbackInfo &info)
+{
+    PLOG_DEBUG << "close() called";
+    doClose();
 }
 
-void PeerConnectionWrapper::destroy(const Napi::CallbackInfo &info)
+void PeerConnectionWrapper::doCleanup()
 {
-    PLOG_DEBUG << "destroy() called";
-    doDestroy();
+    PLOG_DEBUG << "doCleanup() called";
+    mOnStateChangeCallback.reset();
+    instances.erase(this);
 }
 
 void PeerConnectionWrapper::setLocalDescription(const Napi::CallbackInfo &info)
@@ -738,6 +725,13 @@ void PeerConnectionWrapper::onStateChange(const Napi::CallbackInfo &info)
                 stream << state;
                 args = {Napi::String::New(env, stream.str())};
                 PLOG_DEBUG << "mOnStateChangeCallback call(2)";
+            },[this,state](){
+                PLOG_DEBUG << "mOnStateChangeCallback cleanup";
+                // Special case for closed state, we need to reset all callbacks
+                if(state == rtc::PeerConnection::State::Closed)
+                {
+                    doCleanup();
+                }
             }); });
 }
 

src/peer-connection-wrapper.h

@@ -19,8 +19,6 @@ class PeerConnectionWrapper : public Napi::ObjectWrap<PeerConnectionWrapper>
   PeerConnectionWrapper(const Napi::CallbackInfo &info);
   ~PeerConnectionWrapper();
 
-  void destroy(const Napi::CallbackInfo &info);
-
   // Functions
   void close(const Napi::CallbackInfo &info);
   void setLocalDescription(const Napi::CallbackInfo &info);
@@ -58,15 +56,14 @@ class PeerConnectionWrapper : public Napi::ObjectWrap<PeerConnectionWrapper>
   static void CloseAll();
 
   // Reset all Callbacks for existing Peer Connections
-  static void ResetCallbacksAll();
+  static void CleanupAll();
 
 private:
   static Napi::FunctionReference constructor;
   static std::unordered_set<PeerConnectionWrapper *> instances;
 
   void doClose();
-  void doDestroy();
-  void doResetCallbacks();
+  void doCleanup();
 
   std::string mPeerName;
   std::unique_ptr<rtc::PeerConnection> mRtcPeerConnPtr = nullptr;

src/rtc-wrapper.cpp

@@ -125,8 +125,11 @@ void RtcWrapper::cleanup(const Napi::CallbackInfo &info)
         if (rtc::Cleanup().wait_for(std::chrono::seconds(timeout)) == std::future_status::timeout)
             throw std::runtime_error("cleanup timeout (possible deadlock)");
 
-        // Clear Callbacks
-        PeerConnectionWrapper::ResetCallbacksAll();
+        // Cleanup the instances
+        PeerConnectionWrapper::CleanupAll();
+        DataChannelWrapper::CleanupAll();
+        TrackWrapper::CleanupAll();
+        WebSocketWrapper::CleanupAll();
 
         if (logCallback)
             logCallback.reset();

src/thread-safe-callback.cpp

@@ -26,9 +26,9 @@ ThreadSafeCallback::~ThreadSafeCallback()
     tsfn.Abort();
 }
 
-void ThreadSafeCallback::call(arg_func_t argFunc)
+void ThreadSafeCallback::call(arg_func_t argFunc, cleanup_func_t cleanupFunc)
 {
-    CallbackData *data = new CallbackData{std::move(argFunc)};
+    CallbackData *data = new CallbackData{std::move(argFunc), std::move(cleanupFunc)};
     if (tsfn.BlockingCall(data) != napi_ok)
     {
         delete data;
@@ -47,6 +47,7 @@ void ThreadSafeCallback::callbackFunc(Napi::Env env,
 
     arg_vector_t args;
     arg_func_t argFunc(std::move(data->argFunc));
+    cleanup_func_t cleanup(std::move(data->cleanupFunc));
     delete data;
 
     try
@@ -59,5 +60,8 @@ void ThreadSafeCallback::callbackFunc(Napi::Env env,
     }
 
     if (env && callback)
+    {
         callback.Call(context->Value(), args);
+        cleanup();
+    }
 }