fix: parameter and return type bugs in peer connection wrapper

[mertushka]

No description provided.

[mertushka]

node-datachannel/src/cpp/peer-connection-wrapper.cpp

Line 585 in d83ba00

init.reliability.unordered = !initConfig.Get("unordered").As<Napi::Boolean>();

@murat-dogan noticed the unordered parameter uses negation (!). Is this intentional or we should fix that too?

Also,

node-datachannel/src/polyfill/RTCPeerConnection.ts

Lines 364 to 365 in d83ba00

	createDataChannel(label: string, opts: globalThis.RTCDataChannelInit = {}): RTCDataChannel {
	const channel = this.#peerConnection.createDataChannel(label, opts);

The polyfill expects globalThis.RTCDataChannelInit (which has ordered) but passes it directly to the native layer which expects DataChannelInitConfig (which has unordered). This causes a type mismatch (i don't know why typescript doesn't warns us about that?) where the native layer doesn't receive the ordering option at all.

Should I create a separate PR to fix the polyfill layer mapping?

[mertushka]

Critical bug (may be related with #366)

Found a serious memory safety issue in createDataChannel (and also onDataChannel, addTrack, onTrack):

node-datachannel/src/cpp/peer-connection-wrapper.cpp

Lines 620 to 622 in d83ba00

	std::shared_ptr<rtc::DataChannel> dataChannel = mRtcPeerConnPtr->createDataChannel(label, std::move(init));
	auto instance = DataChannelWrapper::constructor.New(
	{Napi::External<std::shared_ptr<rtc::DataChannel>>::New(info.Env(), &dataChannel)});

The dataChannel variable gets destroyed when the function returns, but we're passing &dataChannel to Napi::External. This means the JS wrapper holds a pointer to freed stack memory. When JS tries to use that External value later, it reads from freed memory, undefined behavior that can cause random crashes.

Instead of passing a pointer to a local stack variable, we need to heap-allocate the shared_ptr and give that to External with a proper finalizer.

Fix for createDataChannel:

auto dataChannel = mRtcPeerConnPtr->createDataChannel(label, init);
// Allocate a *heap* copy of the shared_ptr
auto heapPtr = new std::shared_ptr<rtc::DataChannel>(dataChannel);
// Pass heapPtr into External, and tell N-API how to clean it up
auto instance = DataChannelWrapper::constructor.New({
   Napi::External<std::shared_ptr<rtc::DataChannel>>::New(
       info.Env(),
       heapPtr,
       [](Napi::Env /*env*/, std::shared_ptr<rtc::DataChannel>* p) {
           delete p; // deletes the heap-allocated shared_ptr
       }
   )
});

This way:

• The heap object survives after the function returns
• The shared_ptr still manages the underlying rtc::DataChannel lifetime
• When JS drops the object, the finalizer deletes the heap-allocated shared_ptr

Same fix needed for onDataChannel, addTrack, onTrack - anywhere we do Napi::External<std::shared_ptr<X>>::New(env, &localSharedPtr).

[mertushka]

@murat-dogan just checking in to see if you had a chance to review this PR.

[murat-dogan]

Hello @mertushka ,

I am on holiday and will check them as soon as when I return.

[mertushka]

@murat-dogan Knew it, enjoy your holiday tatil! 😁

[murat-dogan]

Thanks.

I need to check this separately
#368 (comment)