Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Worker: Fix sending callback leaks #1383

Merged
merged 5 commits into from
Apr 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 11 additions & 10 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
### NEXT

- Update worker subprojects ([PR #1376](https://github.com/versatica/mediasoup/pull/1376)).
- OPUS: Fix DTX detection #1357 ([PR #1357](https://github.com/versatica/mediasoup/pull/1357)).
- OPUS: Fix DTX detection ([PR #1357](https://github.com/versatica/mediasoup/pull/1357)).
- Worker: Fix sending callback leaks ([PR #1383](https://github.com/versatica/mediasoup/pull/1383), credits to @Lynnworld).

### 3.14.1

Expand Down Expand Up @@ -72,13 +73,13 @@

### 3.13.13

- worker: Do not use references for async callbacks ([PR #1274](https://github.com/versatica/mediasoup/pull/1274)).
- Worker: Do not use references for async callbacks ([PR #1274](https://github.com/versatica/mediasoup/pull/1274)).
- liburing: Enable zero copy ([PR #1273](https://github.com/versatica/mediasoup/pull/1273)).
- Fix build on musl based systems (such as Alpine Linux) ([PR #1279](https://github.com/versatica/mediasoup/pull/1279)).

### 3.13.12

- worker: Disable `RtcLogger` usage if not enabled ([PR #1264](https://github.com/versatica/mediasoup/pull/1264)).
- Worker: Disable `RtcLogger` usage if not enabled ([PR #1264](https://github.com/versatica/mediasoup/pull/1264)).
- npm installation: Don't require Python if valid worker prebuilt binary is fetched ([PR #1265](https://github.com/versatica/mediasoup/pull/1265)).
- Update h264-profile-level-id NPM dependency to 1.1.0.

Expand Down Expand Up @@ -144,7 +145,7 @@
- Add optional `rtcpListenInfo` in `PlainTransportOptions` ([PR #1099](https://github.com/versatica/mediasoup/pull/1099)).
- Add pause/resume API in `DataProducer` and `DataConsumer` ([PR #1104](https://github.com/versatica/mediasoup/pull/1104)).
- DataChannel subchannels feature ([PR #1152](https://github.com/versatica/mediasoup/pull/1152)).
- `Worker`: Make DTLS fragment stay within MTU size range ([PR #1156](https://github.com/versatica/mediasoup/pull/1156), based on [PR #1143](https://github.com/versatica/mediasoup/pull/1143) by @vpnts-se).
- Worker: Make DTLS fragment stay within MTU size range ([PR #1156](https://github.com/versatica/mediasoup/pull/1156), based on [PR #1143](https://github.com/versatica/mediasoup/pull/1143) by @vpnts-se).

### 3.12.16

Expand Down Expand Up @@ -189,7 +190,7 @@

### 3.12.6

- `Worker`: Add `Transport::Destroying()` protected method ([PR #1114](https://github.com/versatica/mediasoup/pull/1114)).
- Worker: Add `Transport::Destroying()` protected method ([PR #1114](https://github.com/versatica/mediasoup/pull/1114)).
- `RtpStreamRecv`: Fix jitter calculation ([PR #1117](https://github.com/versatica/mediasoup/pull/1117), thanks to @penguinol).
- Revert "Node: make types.ts only export types rather than the entire class/code" ([PR #1109](https://github.com/versatica/mediasoup/pull/1109)) because it requires `typescript` >= 5 in the apps that import mediasoup and we don't want to be that strict yet.

Expand Down Expand Up @@ -219,11 +220,11 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.11.26

- `Worker`: Fix NACK timer and avoid negative RTT ([PR #1082](https://github.com/versatica/mediasoup/pull/1082), thanks to @o-u-p for his work in ([PR #1076](https://github.com/versatica/mediasoup/pull/1076)).
- Worker: Fix NACK timer and avoid negative RTT ([PR #1082](https://github.com/versatica/mediasoup/pull/1082), thanks to @o-u-p for his work in ([PR #1076](https://github.com/versatica/mediasoup/pull/1076)).

### 3.11.25

- `Worker`: Require C++17, Meson >= 1.1.0 and update subprojects ([PR #1081](https://github.com/versatica/mediasoup/pull/1081)).
- Worker: Require C++17, Meson >= 1.1.0 and update subprojects ([PR #1081](https://github.com/versatica/mediasoup/pull/1081)).

### 3.11.24

Expand Down Expand Up @@ -323,7 +324,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

- Node: Migrate tests to TypeScript ([PR #958](https://github.com/versatica/mediasoup/pull/958)).
- Node: Remove compiled JavaScript from repository and compile TypeScript code on NPM `prepare` script on demand when installed via git ([PR #954](https://github.com/versatica/mediasoup/pull/954)).
- `Worker`: Add `RTC::Shared` singleton for RTC entities ([PR #953](https://github.com/versatica/mediasoup/pull/953)).
- Worker: Add `RTC::Shared` singleton for RTC entities ([PR #953](https://github.com/versatica/mediasoup/pull/953)).
- Update OpenSSL to 3.0.7.

### 3.11.3
Expand Down Expand Up @@ -499,7 +500,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi

### 3.9.4

- `Worker`: Fix bad printing of error messages from Worker ([PR #750](https://github.com/versatica/mediasoup/pull/750) by @j1elo).
- Worker: Fix bad printing of error messages from Worker ([PR #750](https://github.com/versatica/mediasoup/pull/750) by @j1elo).

### 3.9.3

Expand All @@ -517,7 +518,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi
### 3.9.1

- NixOS friendly build process ([PR #683](https://github.com/versatica/mediasoup/pull/683)).
- `Worker`: Emit "died" event before observer "close" ([PR #684](https://github.com/versatica/mediasoup/pull/684)).
- Worker: Emit "died" event before observer "close" ([PR #684](https://github.com/versatica/mediasoup/pull/684)).
- Transport: Hide debug message for RTX RTCP-RR packets ([PR #688](https://github.com/versatica/mediasoup/pull/688)).
- Update `libuv` to 1.42.0.
- Improve Windows support ([PR #692](https://github.com/versatica/mediasoup/pull/692)).
Expand Down
24 changes: 24 additions & 0 deletions worker/src/RTC/DataConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,12 @@ namespace RTC

if (!IsActive())
{
if (cb)
{
(*cb)(false, false);
delete cb;
}

return;
}

Expand All @@ -536,6 +542,12 @@ namespace RTC
requiredSubchannel.has_value() &&
this->subchannels.find(requiredSubchannel.value()) == this->subchannels.end())
{
if (cb)
{
(*cb)(false, false);
delete cb;
}

return;
}

Expand All @@ -557,6 +569,12 @@ namespace RTC

if (!subchannelMatched)
{
if (cb)
{
(*cb)(false, false);
delete cb;
}

return;
}
}
Expand All @@ -569,6 +587,12 @@ namespace RTC
len,
this->maxMessageSize);

if (cb)
{
(*cb)(false, false);
delete cb;
}

return;
}

Expand Down
18 changes: 13 additions & 5 deletions worker/src/RTC/DirectTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ namespace RTC
{
MS_WARN_TAG(rtp, "cannot send RTP packet not associated to a Consumer");

if (cb)
{
(*cb)(false);
delete cb;
}

return;
}

Expand Down Expand Up @@ -212,11 +218,7 @@ namespace RTC
}

void DirectTransport::SendMessage(
RTC::DataConsumer* dataConsumer,
const uint8_t* msg,
size_t len,
uint32_t ppid,
onQueuedCallback* /*cb*/)
RTC::DataConsumer* dataConsumer, const uint8_t* msg, size_t len, uint32_t ppid, onQueuedCallback* cb)
{
MS_TRACE();

Expand All @@ -232,6 +234,12 @@ namespace RTC
FBS::Notification::Body::DataConsumer_MessageNotification,
notification);

if (cb)
{
(*cb)(true, false);
delete cb;
}

// Increase send transmission.
RTC::Transport::DataSent(len);
}
Expand Down