Possible per-call heap leak of `tr_param` in `Client::configure`

Open Beginner friendly
#130 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
cpp, nodejs

Research direction

Start in src/rtms.cpp at Client::configure and trace the MediaParams::toNative call and the native_params cleanup block. Exercise configure repeatedly with transcript and another media parameter enabled, then verify that every allocated native parameter is released on normal and error paths. Done means the repeated configuration no longer accumulates transcript_parameters allocations.

Written by the indexing model from the issue text.

Description

Possible per-call heap leak of tr_param in Client::configure

I found a possible alloc-before-free / member-handle leak in Client::configure. MediaParams::toNative()
heap-allocates up to four native parameter structs (audio_param, video_param, ds_param, tr_param)
and stores raw pointers into a plain C media_parameters struct. After calling sdk_->config(&native_params, ...),
configure() frees only audio_param, video_param, and ds_param — it never frees tr_param. So every
configure() call made while transcript params (plus at least one of audio/video/deskshare) are set leaks one
heap-allocated transcript_parameters object. configure() is invoked per operation (from join(),
enableAudio/Video/Deskshare, setAudioParams, etc.), so the leak recurs for the life of the client.

File: src/rtms.cpp

Function: Client::configure (and MediaParams::toNative)

// MediaParams::toNative() — allocates all four, including tr_param:
if (transcript_params_) {
    transcript_parameters* tr_params = new transcript_parameters();
    *tr_params = transcript_params_->toNative();
    params.tr_param = tr_params;
}
...
// Client::configure():
media_parameters native_params = media_params_.toNative();
...
int result = sdk_->config(&native_params, media_types, enable_application_layer_encryption ? 1 : 0);

if (native_params.audio_param) { delete native_params.audio_param; }
if (native_params.video_param) { delete native_params.video_param; }
if (native_params.ds_param)    { delete native_params.ds_param; }
// <-- no `delete native_params.tr_param;` — leaked
throwIfError(result, "configure");
  1. media_params_.toNative() is called only from configure() (verified: line 589 is the sole caller).
  2. When transcript params are configured, toNative() does new transcript_parameters() and stores it in
    native_params.tr_param.
  3. media_parameters is a plain C SDK struct with no destructor, so the raw pointer is not auto-freed.
  4. The cleanup block deletes audio_param, video_param, and ds_param but omits tr_param.
  5. native_params then goes out of scope; the transcript_parameters object is unreachable and leaks.
  6. The early "null params" branch returns before toNative(), so it does not leak — the leak requires at least
    one non-transcript media type set together with transcript params (a normal RTMS transcription setup).

JS trigger (if applicable):

const client = new rtms.Client();
client.setOnAudioData(() => {});
client.setOnTranscriptData(() => {});
// enabling audio while transcript params are set -> configure() -> leaks a transcript_parameters each time
client.enableAudio(true);
client.enableAudio(false);
client.enableAudio(true); // repeat = repeated leaks

Suggested fix: add if (native_params.tr_param) { delete native_params.tr_param; } alongside the other three
deletes (or, better, have toNative() return an RAII owner so all four are freed uniformly).

Dominant language
C++
Stars
41
Forks
20
PR merge metrics
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from zoom/rtms

All issues in zoom/rtms

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.