<p>Walter Doekes <strong>uploaded patch set #3</strong> to this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/10991">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">sched: Don't allow ast_sched_del to deadlock ast_sched_runq from same thread<br><br>When fixing ASTERISK-24212, a change was done so a scheduled callback could not<br>be removed while it was running. The caller of ast_sched_del would have to wait.<br><br>However, when the caller of ast_sched_del is the callback itself (however wrong<br>this might be), this new check would cause a deadlock: it would wait forever<br>for itself.<br><br>This changeset introduces an additional check: if ast_sched_del is called<br>by the callback itself, it is immediately rejected (along with an ERROR log and<br>a backtrace). Additionally, the AST_SCHED_DEL_UNREF macro is adjusted so the<br>after-ast_sched_del-refcall function is only run if ast_sched_del returned<br>success.<br><br>This should fix the following spurious race condition found in chan_sip:<br>- thread 1: schedule sip_poke_peer_now (using AST_SCHED_REPLACE)<br>- thread 2: run sip_poke_peer_now<br>- thread 2: blank out sched-ID (too soon!)<br>- thread 1: set sched-ID (too late!)<br>- thread 2: try to delete the currently running sched-ID<br><br>After this fix, an ERROR would be logged, but no deadlocks (in do_monitor) nor<br>excess calls to sip_unref_peer(peer) (causing double frees of rtp_instances and<br>other madness) should occur.<br><br>(Thanks Richard Mudgett for reviewing/improving this "scary" change.)<br><br>Note that this change does not fix the observed race condition: unlocked<br>access to peer->pokeexpire (and potentially other scheduled items in chan_sip),<br>causing AST_SCHED_DEL_UNREF to look at a changing id. But it will make the<br>deadlock go away. And in the observed case, it will not have adverse affects<br>(like memory leaks) because the scheduled item is removed through a different<br>path.<br><br>ASTERISK-28282<br><br>Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856<br>---<br>M include/asterisk/sched.h<br>M main/sched.c<br>2 files changed, 32 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/91/10991/3</pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/10991">change 10991</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/+/10991"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856 </div>
<div style="display:none"> Gerrit-Change-Number: 10991 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Walter Doekes <walter+asterisk@wjd.nu> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Walter Doekes <walter+asterisk@wjd.nu> </div>
<div style="display:none"> Gerrit-MessageType: newpatchset </div>