Skip to content

Commit 1ec56d1

Browse files
murat-doganmurat-dogan
authored
Merge pull request #239 from murat-dogan/close-wıthout-destroy
Close without destroy
2 parents 930b858 + 0abeeb2 commit 1ec56d1

23 files changed

+447
-309
lines changed

‎CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.15)
22
cmake_policy(SET CMP0091 NEW)
33
cmake_policy(SET CMP0042 NEW)
44

5-
project(node_datachannel VERSION 0.6.0)
5+
project(node_datachannel VERSION 0.7.0)
66

77
# -Dnapi_build_version=8
88
add_definitions(-DNAPI_VERSION=8)

‎jest.config.cjs

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/** @type {import('jest').Config} */
2+
const config = {
3+
verbose: true,
4+
testPathIgnorePatterns: ['<rootDir>/node_modules/', 'multiple-run.test'],
5+
};
6+
7+
module.exports = config;

‎lib/index.d.ts

-1
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ export class WebSocketServer {
275275
export class PeerConnection {
276276
constructor(peerName: string, config: RtcConfig);
277277
close(): void;
278-
destroy(): void;
279278
setLocalDescription(type?: DescriptionType): void;
280279
setRemoteDescription(sdp: string, type: DescriptionType): void;
281280
localDescription(): { type: string; sdp: string } | null;

‎package-lock.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "node-datachannel",
3-
"version": "0.6.0",
3+
"version": "0.7.0-dev",
44
"description": "libdatachannel node bindings",
55
"type": "module",
66
"exports": {

‎polyfill/RTCPeerConnection.js

-1
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,6 @@ export default class _RTCPeerConnection extends EventTarget {
222222
});
223223

224224
this.#peerConnection.close();
225-
this.#peerConnection.destroy();
226225
}
227226

228227
createAnswer() {

‎src/data-channel-wrapper.cpp

+15-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ void DataChannelWrapper::CloseAll()
1313
inst->doClose();
1414
}
1515

16+
void DataChannelWrapper::CleanupAll()
17+
{
18+
PLOG_DEBUG << "CleanupAll() called";
19+
auto copy(instances);
20+
for (auto inst : copy)
21+
inst->doCleanup();
22+
}
23+
1624
Napi::Object DataChannelWrapper::Init(Napi::Env env, Napi::Object exports)
1725
{
1826
Napi::HandleScope scope(env);
@@ -78,11 +86,15 @@ void DataChannelWrapper::doClose()
7886
}
7987

8088
mOnOpenCallback.reset();
81-
mOnClosedCallback.reset();
8289
mOnErrorCallback.reset();
8390
mOnBufferedAmountLowCallback.reset();
8491
mOnMessageCallback.reset();
92+
}
8593

94+
void DataChannelWrapper::doCleanup()
95+
{
96+
PLOG_DEBUG << "doCleanup() called";
97+
mOnClosedCallback.reset();
8698
instances.erase(this);
8799
}
88100

@@ -350,6 +362,8 @@ void DataChannelWrapper::onClosed(const Napi::CallbackInfo &info)
350362
// This will run in main thread and needs to construct the
351363
// arguments for the call
352364
args = {};
365+
},[this]{
366+
doCleanup();
353367
}); });
354368
}
355369

‎src/data-channel-wrapper.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,14 @@ class DataChannelWrapper : public Napi::ObjectWrap<DataChannelWrapper>
4242
// Close all existing DataChannels
4343
static void CloseAll();
4444

45+
// Reset all Callbacks for existing DataChannels
46+
static void CleanupAll();
47+
4548
private:
46-
static std::unordered_set<DataChannelWrapper*> instances;
49+
static std::unordered_set<DataChannelWrapper *> instances;
4750

4851
void doClose();
52+
void doCleanup();
4953

5054
std::string mLabel;
5155
std::shared_ptr<rtc::DataChannel> mDataChannelPtr = nullptr;

‎src/media-track-wrapper.cpp

+25-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
#include "media-direction.h"
33
#include "media-rtcpreceivingsession-wrapper.h"
44

5+
#include "plog/Log.h"
6+
57
Napi::FunctionReference TrackWrapper::constructor;
68
std::unordered_set<TrackWrapper *> TrackWrapper::instances;
79

@@ -12,6 +14,14 @@ void TrackWrapper::CloseAll()
1214
inst->doClose();
1315
}
1416

17+
void TrackWrapper::CleanupAll()
18+
{
19+
PLOG_DEBUG << "CleanupAll() called";
20+
auto copy(instances);
21+
for (auto inst : copy)
22+
inst->doCleanup();
23+
}
24+
1525
Napi::Object TrackWrapper::Init(Napi::Env env, Napi::Object exports)
1626
{
1727
Napi::HandleScope scope(env);
@@ -73,10 +83,14 @@ void TrackWrapper::doClose()
7383
}
7484

7585
mOnOpenCallback.reset();
76-
mOnClosedCallback.reset();
7786
mOnErrorCallback.reset();
7887
mOnMessageCallback.reset();
88+
}
7989

90+
void TrackWrapper::doCleanup()
91+
{
92+
PLOG_DEBUG << "doCleanup() called";
93+
mOnClosedCallback.reset();
8094
instances.erase(this);
8195
}
8296

@@ -276,7 +290,8 @@ void TrackWrapper::onOpen(const Napi::CallbackInfo &info)
276290
// Callback
277291
mOnOpenCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
278292

279-
mTrackPtr->onOpen([&]() {
293+
mTrackPtr->onOpen([&]()
294+
{
280295
if (mOnOpenCallback)
281296
mOnOpenCallback->call([this](Napi::Env env, std::vector<napi_value> &args) {
282297
// Check the track is not closed
@@ -309,14 +324,17 @@ void TrackWrapper::onClosed(const Napi::CallbackInfo &info)
309324
// Callback
310325
mOnClosedCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
311326

312-
mTrackPtr->onClosed([&]() {
327+
mTrackPtr->onClosed([&]()
328+
{
313329
if (mOnClosedCallback)
314330
mOnClosedCallback->call([this](Napi::Env env, std::vector<napi_value> &args) {
315331
// Do not check if the data channel has been closed here
316332

317333
// This will run in main thread and needs to construct the
318334
// arguments for the call
319335
args = {};
336+
},[this]{
337+
doCleanup();
320338
}); });
321339
}
322340

@@ -340,7 +358,8 @@ void TrackWrapper::onError(const Napi::CallbackInfo &info)
340358
// Callback
341359
mOnErrorCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
342360

343-
mTrackPtr->onError([&](const std::string &error) {
361+
mTrackPtr->onError([&](const std::string &error)
362+
{
344363
if (mOnErrorCallback)
345364
mOnErrorCallback->call([this, error](Napi::Env env, std::vector<napi_value> &args) {
346365
// Check the track is not closed
@@ -373,7 +392,8 @@ void TrackWrapper::onMessage(const Napi::CallbackInfo &info)
373392
// Callback
374393
mOnMessageCallback = std::make_unique<ThreadSafeCallback>(info[0].As<Napi::Function>());
375394

376-
mTrackPtr->onMessage([&](std::variant<rtc::binary, std::string> message) {
395+
mTrackPtr->onMessage([&](std::variant<rtc::binary, std::string> message)
396+
{
377397
if (mOnMessageCallback)
378398
mOnMessageCallback->call([this, message = std::move(message)](Napi::Env env, std::vector<napi_value> &args) {
379399
// Check the track is not closed

‎src/media-track-wrapper.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,25 @@ class TrackWrapper : public Napi::ObjectWrap<TrackWrapper>
3939
void onError(const Napi::CallbackInfo &info);
4040
void onMessage(const Napi::CallbackInfo &info);
4141

42-
// Close all existing track
42+
// Close all existing tracks
4343
static void CloseAll();
4444

45+
// Reset all Callbacks for existing tracks
46+
static void CleanupAll();
47+
4548
private:
4649
static std::unordered_set<TrackWrapper *> instances;
4750

4851
void doClose();
52+
void doCleanup();
4953

5054
std::shared_ptr<rtc::Track> mTrackPtr = nullptr;
5155

52-
// Callback Ptrs
56+
// Callback Ptrs
5357
std::unique_ptr<ThreadSafeCallback> mOnOpenCallback = nullptr;
5458
std::unique_ptr<ThreadSafeCallback> mOnClosedCallback = nullptr;
5559
std::unique_ptr<ThreadSafeCallback> mOnErrorCallback = nullptr;
5660
std::unique_ptr<ThreadSafeCallback> mOnMessageCallback = nullptr;
5761
};
5862

59-
#endif // MEDIA_TRACK_WRAPPER_H
63+
#endif // MEDIA_TRACK_WRAPPER_H

‎src/peer-connection-wrapper.cpp

+21-27
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ void PeerConnectionWrapper::CloseAll()
2020
inst->doClose();
2121
}
2222

23-
void PeerConnectionWrapper::ResetCallbacksAll()
23+
void PeerConnectionWrapper::CleanupAll()
2424
{
25-
PLOG_DEBUG << "ResetCallbacksAll() called";
25+
PLOG_DEBUG << "CleanupAll() called";
2626
auto copy(instances);
2727
for (auto inst : copy)
28-
inst->doResetCallbacks();
28+
inst->doCleanup();
2929
}
3030

3131
Napi::Object PeerConnectionWrapper::Init(Napi::Env env, Napi::Object exports)
@@ -37,7 +37,6 @@ Napi::Object PeerConnectionWrapper::Init(Napi::Env env, Napi::Object exports)
3737
"PeerConnection",
3838
{
3939
InstanceMethod("close", &PeerConnectionWrapper::close),
40-
InstanceMethod("destroy", &PeerConnectionWrapper::destroy),
4140
InstanceMethod("setLocalDescription", &PeerConnectionWrapper::setLocalDescription),
4241
InstanceMethod("setRemoteDescription", &PeerConnectionWrapper::setRemoteDescription),
4342
InstanceMethod("localDescription", &PeerConnectionWrapper::localDescription),
@@ -253,7 +252,8 @@ PeerConnectionWrapper::PeerConnectionWrapper(const Napi::CallbackInfo &info) : N
253252
PeerConnectionWrapper::~PeerConnectionWrapper()
254253
{
255254
PLOG_DEBUG << "Destructor called";
256-
doDestroy();
255+
doCleanup();
256+
doClose();
257257
}
258258

259259
void PeerConnectionWrapper::doClose()
@@ -273,40 +273,27 @@ void PeerConnectionWrapper::doClose()
273273
return;
274274
}
275275
}
276-
}
277-
278-
void PeerConnectionWrapper::close(const Napi::CallbackInfo &info)
279-
{
280-
PLOG_DEBUG << "close() called";
281-
doClose();
282-
}
283-
284-
void PeerConnectionWrapper::doDestroy()
285-
{
286-
PLOG_DEBUG << "doDestroy() called";
287-
doClose();
288-
doResetCallbacks();
289-
}
290276

291-
void PeerConnectionWrapper::doResetCallbacks()
292-
{
293-
PLOG_DEBUG << "doResetCallbacks() called";
294277
mOnLocalDescriptionCallback.reset();
295278
mOnLocalCandidateCallback.reset();
296-
mOnStateChangeCallback.reset();
297279
mOnIceStateChangeCallback.reset();
298280
mOnSignalingStateChangeCallback.reset();
299281
mOnGatheringStateChangeCallback.reset();
300282
mOnDataChannelCallback.reset();
301283
mOnTrackCallback.reset();
284+
}
302285

303-
instances.erase(this);
286+
void PeerConnectionWrapper::close(const Napi::CallbackInfo &info)
287+
{
288+
PLOG_DEBUG << "close() called";
289+
doClose();
304290
}
305291

306-
void PeerConnectionWrapper::destroy(const Napi::CallbackInfo &info)
292+
void PeerConnectionWrapper::doCleanup()
307293
{
308-
PLOG_DEBUG << "destroy() called";
309-
doDestroy();
294+
PLOG_DEBUG << "doCleanup() called";
295+
mOnStateChangeCallback.reset();
296+
instances.erase(this);
310297
}
311298

312299
void PeerConnectionWrapper::setLocalDescription(const Napi::CallbackInfo &info)
@@ -738,6 +725,13 @@ void PeerConnectionWrapper::onStateChange(const Napi::CallbackInfo &info)
738725
stream << state;
739726
args = {Napi::String::New(env, stream.str())};
740727
PLOG_DEBUG << "mOnStateChangeCallback call(2)";
728+
},[this,state](){
729+
PLOG_DEBUG << "mOnStateChangeCallback cleanup";
730+
// Special case for closed state, we need to reset all callbacks
731+
if(state == rtc::PeerConnection::State::Closed)
732+
{
733+
doCleanup();
734+
}
741735
}); });
742736
}
743737

‎src/peer-connection-wrapper.h

+2-5
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ class PeerConnectionWrapper : public Napi::ObjectWrap<PeerConnectionWrapper>
1919
PeerConnectionWrapper(const Napi::CallbackInfo &info);
2020
~PeerConnectionWrapper();
2121

22-
void destroy(const Napi::CallbackInfo &info);
23-
2422
// Functions
2523
void close(const Napi::CallbackInfo &info);
2624
void setLocalDescription(const Napi::CallbackInfo &info);
@@ -58,15 +56,14 @@ class PeerConnectionWrapper : public Napi::ObjectWrap<PeerConnectionWrapper>
5856
static void CloseAll();
5957

6058
// Reset all Callbacks for existing Peer Connections
61-
static void ResetCallbacksAll();
59+
static void CleanupAll();
6260

6361
private:
6462
static Napi::FunctionReference constructor;
6563
static std::unordered_set<PeerConnectionWrapper *> instances;
6664

6765
void doClose();
68-
void doDestroy();
69-
void doResetCallbacks();
66+
void doCleanup();
7067

7168
std::string mPeerName;
7269
std::unique_ptr<rtc::PeerConnection> mRtcPeerConnPtr = nullptr;

‎src/rtc-wrapper.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,11 @@ void RtcWrapper::cleanup(const Napi::CallbackInfo &info)
125125
if (rtc::Cleanup().wait_for(std::chrono::seconds(timeout)) == std::future_status::timeout)
126126
throw std::runtime_error("cleanup timeout (possible deadlock)");
127127

128-
// Clear Callbacks
129-
PeerConnectionWrapper::ResetCallbacksAll();
128+
// Cleanup the instances
129+
PeerConnectionWrapper::CleanupAll();
130+
DataChannelWrapper::CleanupAll();
131+
TrackWrapper::CleanupAll();
132+
WebSocketWrapper::CleanupAll();
130133

131134
if (logCallback)
132135
logCallback.reset();

‎src/thread-safe-callback.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ ThreadSafeCallback::~ThreadSafeCallback()
2626
tsfn.Abort();
2727
}
2828

29-
void ThreadSafeCallback::call(arg_func_t argFunc)
29+
void ThreadSafeCallback::call(arg_func_t argFunc, cleanup_func_t cleanupFunc)
3030
{
31-
CallbackData *data = new CallbackData{std::move(argFunc)};
31+
CallbackData *data = new CallbackData{std::move(argFunc), std::move(cleanupFunc)};
3232
if (tsfn.BlockingCall(data) != napi_ok)
3333
{
3434
delete data;
@@ -47,6 +47,7 @@ void ThreadSafeCallback::callbackFunc(Napi::Env env,
4747

4848
arg_vector_t args;
4949
arg_func_t argFunc(std::move(data->argFunc));
50+
cleanup_func_t cleanup(std::move(data->cleanupFunc));
5051
delete data;
5152

5253
try
@@ -59,5 +60,8 @@ void ThreadSafeCallback::callbackFunc(Napi::Env env,
5960
}
6061

6162
if (env && callback)
63+
{
6264
callback.Call(context->Value(), args);
65+
cleanup();
66+
}
6367
}

0 commit comments

Comments
 (0)