[Asterisk-code-review] sched: Don't allow ast sched del to deadlock ast sched runq ... (asterisk[13])

Walter Doekes asteriskteam at digium.com
Thu Feb 14 01:30:00 CST 2019


Hello Friendly Automation, 

I'd like you to reexamine a change. Please visit

    https://gerrit.asterisk.org/10991

to look at the new patch set (#2).

Change subject: sched: Don't allow ast_sched_del to deadlock ast_sched_runq from same thread
......................................................................

sched: Don't allow ast_sched_del to deadlock ast_sched_runq from same thread

When fixing ASTERISK~24212, a change was done so a scheduled callback could not
be removed while it was running. The caller of ast_sched_del would have to wait.

However, when the caller of ast_sched_del is the callback itself (however wrong
this might be), this new check would cause a deadlock: it would wait forever
for itself.

This changeset introduces an additional check: if ast_sched_del is called
by the callback itself, it is immediately rejected (along with an ERROR log and
a backtrace). Additionally, the AST_SCHED_DEL_UNREF macro is adjusted so the
after-ast_sched_del-refcall function is only run if ast_sched_del returned
success.

This should fix the following spurious race condition found in chan_sip:
- thread 1: schedule sip_poke_peer_now (using AST_SCHED_REPLACE)
- thread 2: run sip_poke_peer_now
- thread 2: blank out sched-ID (too soon!)
- thread 1: set sched-ID (too late!)
- thread 2: try to delete the currently running sched-ID

After this fix, an ERROR would be logged, but no deadlocks (in do_monitor) nor
excess calls to sip_unref_peer(peer) (causing double frees of rtp_instances and
other madness) should occur.

ASTERISK-28282

Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856
---
M include/asterisk/sched.h
M main/sched.c
2 files changed, 25 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/91/10991/2
-- 
To view, visit https://gerrit.asterisk.org/10991
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856
Gerrit-Change-Number: 10991
Gerrit-PatchSet: 2
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190214/0bb13fc5/attachment.html>


More information about the asterisk-code-review mailing list