[Asterisk-code-review] sched: Don't allow ast_sched_del to deadlock ast_sched_runq from same... (...asterisk[13])

[Walter Doekes]

Walter Doekes has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/10991 )

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


Patch Set 2:

I'm sorry, I mixed up initid and waitid in my reasoning above:

>     p->waitid = ast_sched_add(sched, wait, sip_reinvite_retry, p);
>     if (p->waitid < 0) {
>         /* Uh Oh.  Expect bad behavior. */
>         dialog_ref(p, "Failed to schedule waitid"); // <-- this is not supposed to happen though, and see [5]
...
>     AST_SCHED_DEL_UNREF(sched, p->initid, dialog_unref(p, "when you delete the initid sched, you should dec the refcount for the stored dialog ptr"));

That should've been:

        AST_SCHED_DEL_UNREF(sched, dialog->waitid,
                dialog_unref(dialog, "Stop scheduled waitid"));

I accidentally started looking at initid while browsing the source. The "waitid" schedule does not do a AST_SCHED_REPLACE_UNREF as well, so it does not mix always-clean-up vs. clean-up-only-if-scheduled semantics, which I incorrectly suggested it did.

The rest of my argument still stands though.


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/10991
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856
Gerrit-Change-Number: 10991
Gerrit-PatchSet: 2
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: mattf <creslin at digium.com>
Gerrit-Comment-Date: Tue, 12 Mar 2019 09:42:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190312/de879006/attachment.html>