Skip to content

Conversation

@orgads
Copy link
Contributor

@orgads orgads commented Jan 30, 2026

No description provided.

@orgads orgads force-pushed the rtp-debug branch 2 times, most recently from 06ad74e to 3c57706 Compare January 30, 2026 06:34
@orgads orgads requested a review from lemenkov January 30, 2026 06:34
Copy link
Member

@lemenkov lemenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. perhaps we could slightly improve it (in a separate PR) - Claude insists there is a TOCTOU issue here:


Race condition (theoretical, not practical)

printHex() and printVector() check fp before acquiring the mutex but use it after:

if (!fp || !note || !string || !rtpcheck_debug)  // no lock
    return;
std::lock_guard lock(mutex);
fprintf(fp, ...);  // fp could be null if close() ran in between

In practice this can't fire because close() is only called from rtpstream_shutdown() after all threads are joined. But it's technically a TOCTOU race. A quick if (!fp) return; after the lock acquisition would make it bulletproof.

Same pattern in open() and close() — the outer if (fp) is unsynchronized. Again harmless in practice since open/close don't race with each other given SIPp's lifecycle.


@lemenkov lemenkov merged commit 62b8afe into SIPp:master Jan 31, 2026
9 checks passed
@orgads orgads deleted the rtp-debug branch January 31, 2026 16:42
@orgads
Copy link
Contributor Author

orgads commented Jan 31, 2026

@lemenkov I prefer rebase/squash over merge. Disabled merge in settings. If you dislike it, let's discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants