<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><p style="white-space: pre-wrap; word-wrap: break-word;">Just commenting here, I'm exploring and brainstorming to see if there's other options for improving this as I always like to reduce channel locking when possible instead of increasing it, unless there are no better options.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not sure what "better options" you are thinking about (scope wise).</p><p style="white-space: pre-wrap; word-wrap: break-word;">Thread A modifies memory locations X and Y while holding lock L (check_for_rtp_changes() in chan_pjsip.c). Thread B (timer, rtp_check_timeout() in res/res_pjsip_sdp_rtp.c) reads those memory locations, potentially at the exact same moment as thread A is modifying them, so it MUST acquire the same lock as thread A before accessing X and Y to see "stable" values and have a deterministic behaviour. (At least using only POSIX stuff, I'm assuming that we do not want to build our own atomic primitives library based on GCC builtins ...)</p><p style="white-space: pre-wrap; word-wrap: break-word;">Granted, the probability that RTP is returning to Asterisk at the exact same moment as a RTP timeout check is running is low - until same automated thing has a connection time that is per chance exactly the same as the RTP-timeout and suddenly it is much more likely to happen. So I think the solution should cover event this corner case.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also, the RTP timeout is on the order of multiple seconds. I don't see the problem locking the channel every 5 seconds for a quick timeout check.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The best I can think of: first, check the timeout as it is now in, i.e., unguarded by the channel lock. If that check says that there *is* a timeout, only then lock the channel and check the timeout (again) and also the direct media situation while holding the lock. This guarantees no false positives and the channel is only locked when it is locked now (on timeout).</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13860">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13860">change 13860</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/13860"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I8b62012be7685849e8fb2b1c5dd39d35313ca2d1 </div>
<div style="display:none"> Gerrit-Change-Number: 13860 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Michael Neuhauser <mike@firmix.at> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-CC: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-CC: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 16 Mar 2020 16:19:30 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>