Fix: rtp_relay memory leak

[Kicey]

Summary

To fix rtp_relay memory leak metioned in #3539.

Details

The rtp_relay_ctx is not released when the opensips receive uas response with status >= 300. In which case the delete request to rtpengine/rtpproxy is sent and session is freed. But the rtp_relay_ctx pointer is not put into dialog. So when dialog destory, it will not release the corresponding rtp_relay_ctx.

Solution

Retrive the dialog and put the ctx pointer in. So that when destroy dialog, the rtp_relay_ctx will be released.

Compatibility

The change don't break other scenarios. It work well on opensips 3.4.11 .

[Kicey]

The following code has registered the function to release rtp_relay_ctx.

opensips/modules/rtp_relay/rtp_relay_ctx.c

Line 1033 in 738adc3

rtp_relay_dlg_ctx_idx = rtp_relay_dlg.dlg_ctx_register_ptr(rtp_relay_ctx_release);

But the pointer is only put into dialog only when uas response with sucessful status.

opensips/modules/rtp_relay/rtp_relay_ctx.c

Line 1693 in 738adc3

RTP_RELAY_PUT_DLG_CTX(dlg, ctx);

opensips/modules/rtp_relay/rtp_relay_ctx.c

Line 1846 in 738adc3

rtp_relay_sess_success(ctx, sess, t, msg);

opensips/modules/rtp_relay/rtp_relay_ctx.c

Lines 1877 to 1887 in 738adc3

	switch (handle_rtp_relay_ctx_leg_reply(ctx, p->rpl, p->req, sess, RTP_RELAY_CALLEE)) {
	case 0:
	rtp_relay_ctx_leg_reply(ctx, p->rpl, t, sess, RTP_RELAY_CALLEE);
	break;
	case 1:
	lock_start_write(rtp_relay_contexts_lock);
	if (list_is_valid(&ctx->list))
	list_del(&ctx->list);
	lock_stop_write(rtp_relay_contexts_lock);
	break;
	}

[razvancrainea]

Thanks for the report and finding this corner case!
I believe your fix is more or less OK, but I don't see the point setting it on failure - why not setting it all the way from the start? not sure what are the implications though, I will have to do some more investigation to figure it out.

[Kicey]

Thanks for the report and finding this corner case! I believe your fix is more or less OK, but I don't see the point setting it on failure - why not setting it all the way from the start? not sure what are the implications though, I will have to do some more investigation to figure it out.

I think this may because the rtp_relay exports the rtp_relay_ctx as pseudo-variables, so it doesn't put the pointer until all fields are ready (such as to_tag). I'm not sure if it will cause another problem if we set it from the start. I'm going to try setting it from the very start.

[razvancrainea]

I double checked the code, and if the context is not moved in the dialog, it remains in the transaction, and gets removed when the transaction is deleted. Are you sure there is actually a leak? Can you send some debug logs so we can track this down?

[Kicey]

I double checked the code, and if the context is not moved in the dialog, it remains in the transaction, and gets removed when the transaction is deleted. Are you sure there is actually a leak? Can you send some debug logs so we can track this down?

I'not familiar with the memory release logic when transaction is deleted. But we've encountered a scenario where OpenSIPS exhibits significant memory consumption without releasing when utilizing the rtp_relay module for call recording.

I located the problem using GDB. Precisely, I print the stack when the rtp_relay_ctx reference counter when its' update. And I don't see the reference counter decreased when the transaction is deleted. Scritp as follow:

break rtp_relay.c:783
commands
    p &(ctx->ref)
    set $addr = &(ctx->ref)
    delete $bpnum
    watch -l *$addr
    commands
        printf "--- BEFORE WRITE ---\n"
        printf "Memory at %p contains: %d\n", $addr, *$addr
        printf "Call stack before write:\n"
        backtrace
        printf "--- END OF CALL STACK ---\n"
        continue
    end
    printf "--- Start context track ---\n"
    continue
end

And get some stack information about normal memory released scenario (response < 300) and unreleased scenario (response >= 300). Of course, from multiple processes, so the logs are not printed in strict chronological order.

• memory released scenario gdb output: https://gist.github.com/Kicey/de222da9ad20da1a7538c6fc218fc5ea
• memory unreleased scenario gdb ouput: https://gist.github.com/Kicey/c8b70627421c701cb44a6d292fb2ee19

Sensitive information such as IP addresses, etc., has been removed.