sched: Avoid a double deref when AST_SCHED_DEL_UNREF is called on an
executing call-back. This is done by adding a new variable 'rescheduled'
to the struct sched which is set in ast_sched_runq and checked in
ast_sched_del_nonrunning. ast_sched_del_nonrunning is a replacement for
now deprecated ast_sched_del which returns a new possible value -2
if called on an executing call-back with rescheduled set. ast_sched_del
is modified to call ast_sched_del_nonrunning to maintain existing code.
AST_SCHED_DEL_UNREF is also updated to look for the -2 in which case it
will not throw a warning or invoke refcall.
test_sched: Add a new unit test sched_test_freebird that will check the
reference count in the resolved scenario.
ASTERISK-29698
Change-Id: Icfb16b3acbc29cf5b4cef74183f7531caaefe21d
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.
(Thanks Richard Mudgett for reviewing/improving this "scary" change.)
Note that this change does not fix the observed race condition: unlocked
access to peer->pokeexpire (and potentially other scheduled items in chan_sip),
causing AST_SCHED_DEL_UNREF to look at a changing id. But it will make the
deadlock go away. And in the observed case, it will not have adverse affects
(like memory leaks) because the scheduled item is removed through a different
path.
ASTERISK-28282
Change-Id: Ic26777fa0732725e6ca7010df17af77a012aa856
In the past there was an assertion in the ast_sched_del function
and in order to ensure it was useful the calling function name,
line number, and filename had to be passed in. This cause the ABI
to be different between dev mode and non-dev mode.
This assertion is no longer present so the special logic can be
removed to make it the same between them both.
Change-Id: Icbc69c801e357d7004efc5cf2ab936d9b83b6ab8
Checking option_debug directly is incorrect as it ignores file/module
specific debug settings. This system-wide change replaces nearly all
direct checks for option_debug with the DEBUG_ATLEAST macro.
Change-Id: Ic342d4799a945dbc40ac085ac142681094a4ebf0
ASTERISK_REGISTER_FILE no longer has any purpose so this commit removes
all traces of it.
Previously exported symbols removed:
* __ast_register_file
* __ast_unregister_file
* ast_complete_source_filename
This also removes the mtx_prof static variable that was declared when
MTX_PROFILE was enabled. This variable was only used in lock.c so it
is now initialized in that file only.
ASTERISK-26480 #close
Change-Id: I1074af07d71f9e159c48ef36631aa432c86f9966
This patch is part of a series to resolve deadlocks in chan_sip.c.
* Updated sched unit test to check new behavior.
ASTERISK-25023
Change-Id: Ib69437327b3cda5e14c4238d9ff91b2531b34ef3
According to the API doxygen a sched ID of 0 is valid. Unfortunately, 0
was never returned historically and several users incorrectly coded usage
of the returned sched ID assuming that 0 was invalid.
ASTERISK-25476
Change-Id: Ib19c7ebb44ec9fd393ef6646dea806d4f34e3a20
It has been observed that on long-running busy systems, a scheduler
context can eventually hit INT_MAX for its assigned IDs and end up
overflowing into a very low negative number. When this occurs, this can
result in odd behaviors, because a negative return is interpreted by
callers as being a failure. However, the item actually was successfully
scheduled. The result may be that a freed item remains in the scheduler,
resulting in a crash at some point in the future.
The scheduler can overflow because every time that an item is added to
the scheduler, a counter is bumped and that counter's current value is
assigned as the new item's ID.
This patch introduces a new method for assigning scheduler IDs. Instead
of assigning from a counter, a queue of available IDs is maintained.
When assigning a new ID, an ID is pulled from the queue. When a
scheduler item is released, its ID is pushed back onto the queue. This
way, IDs may be reused when they become available, and the growth of ID
numbers is directly related to concurrent activity within a scheduler
context rather than the uptime of the system.
Change-Id: I532708eef8f669d823457d7fefdad9a6078b99b2
When deleting a scheduled item if the item in question is currently
executing the ast_sched_del function waits until it has completed.
This is accomplished using ast_cond_wait. Unfortunately the
ast_cond_wait function can suffer from spurious wakeups so the
predicate needs to be checked after it returns to make sure it has
really woken up as a result of being signaled.
This change adds a loop around the ast_cond_wait to make sure that
it only exits when the executing task has really completed.
ASTERISK-25355 #close
Change-Id: I51198270eb0b637c956c61aa409f46283432be61
channels/chan_iax.c: Prevent the deadlock between iax2_hangup and send_lagrq/
send_ping. This deadlock happens because the scheduled task send_lagrq(or
send_ping) starts execution after the call hangup procedure starts but before
it deletes the tasks in the scheduler.
The solution is to delete scheduled lagrq (and ping) task asynchronously
(i.e. schedule AST_SCHED_DEL for these tasks); By this, AST_SCHED_DEL will
be called in a new context (doesn't have callno locked).
This commit also cleans up the procedure of sending LAGRQ and PING.
main/sched.c: Do not assert when deleting non existant entry from scheduler.
This assert seems to be the reason for a lot of awkward code to avoid it.
ASTERISK-24983 #close
Reported by: Y Ateya
Change-Id: I03bec1fc8faacb89630269e935fa667c6d6c080c
Git does not support the ability to replace a token with a version
string during check-in. While it does have support for replacing a
token on clone, this is somewhat sub-optimal: the token is replaced
with the object hash, which is not particularly easy for human
consumption. What's more, in practice, the source file version was often
not terribly useful. Generally, when triaging bugs, the overall version
of Asterisk is far more useful than an individual SVN version of a file. As a
result, this patch removes Asterisk's support for showing source file
versions.
Specifically, it does the following:
* Rename ASTERISK_FILE_VERSION macro to ASTERISK_REGISTER_FILE, and
remove passing the version in with the macro. Other facilities
than 'core show file version' make use of the file names, such as
setting a debug level only on a specific file. As such, the act of
registering source files with the Asterisk core still has use. The
macro rename now reflects the new macro purpose.
* main/asterisk:
- Refactor the file_version structure to reflect that it no longer
tracks a version field.
- Remove the "core show file version" CLI command. Without the file
version, it is no longer useful.
- Remove the ast_file_version_find function. The file version is no
longer tracked.
- Rename ast_register_file_version/ast_unregister_file_version to
ast_register_file/ast_unregister_file, respectively.
* main/manager: Remove value from the Version key of the ModuleCheck
Action. The actual key itself has not been removed, as doing so would
absolutely constitute a backwards incompatible change. However, since
the file version is no longer tracked, there is no need to attempt to
include it in the Version key.
* UPGRADE: Add notes for:
- Modification to the ModuleCheck AMI Action
- Removal of the "core show file version" CLI command
Change-Id: I6cf0ff280e1668bf4957dc21f32a5ff43444a40e
Tasks that were marked for pending deletion in the scheduler would be moved to
the cache for later reuse, but after being recycled the deleted mark wouldn't
be removed resulting in fresh tasks being deleted without reason... and
immediately moved back into the cache where they could be reused again. This
could cause horrendous things to happen in just about anything that used a
scheduler.
ASTERISK-24321 #close
Reported by: Steve Pitts
Review: https://reviewboard.asterisk.org/r/4071/
........
Merged revisions 425503 from http://svn.asterisk.org/svn/asterisk/branches/12
........
Merged revisions 425504 from http://svn.asterisk.org/svn/asterisk/branches/13
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@425505 65c4cc65-6c06-0410-ace0-fbb531ad65f3
When scheduled tasks run, they are removed from the heap (or hashtab).
When a scheduled task is deleted, if the task can't be found in the
heap (or hashtab), an assertion is triggered. If DO_CRASH is enabled,
this assertion causes a crash.
The problem is, sometimes it just so happens that someone attempts
to delete a scheduled task at the time that it is running, leading
to a crash. This change corrects the issue by tracking which task
is currently running. If that task is attempted to be deleted,
then we mark the task, and then wait for the task to complete.
This way, we can be sure to coordinate task deletion and memory
freeing.
ASTERISK-24212
Reported by Matt Jordan
Review: https://reviewboard.asterisk.org/r/3927
........
Merged revisions 422070 from http://svn.asterisk.org/svn/asterisk/branches/12
........
Merged revisions 422071 from http://svn.asterisk.org/svn/asterisk/branches/13
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@422072 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This is a first stab at tweaking the performance profile of the scheduler. Removing
the hashtab usage removes an extra memory allocation when scheduling something and
makes it so rescheduling does not incur any memory allocation at all.
Review: https://reviewboard.asterisk.org/r/3199/
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@407958 65c4cc65-6c06-0410-ace0-fbb531ad65f3
........
r369001 | kpfleming | 2012-06-15 10:56:08 -0500 (Fri, 15 Jun 2012) | 11 lines
Add support-level indications to many more source files.
Since we now have tools that scan through the source tree looking for files
with specific support levels, we need to ensure that every file that is
a component of a 'core' or 'extended' module (or the main Asterisk binary)
is explicitly marked with its support level. This patch adds support-level
indications to many more source files in tree, but avoids adding them to
third-party libraries that are included in the tree and to source files
that don't end up involved in Asterisk itself.
........
r369002 | kpfleming | 2012-06-15 10:57:14 -0500 (Fri, 15 Jun 2012) | 3 lines
Add a script to enable finding source files without support-levels defined.
........
Merged revisions 369001-369002 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 369005 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@369013 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Everything still compiled after making these changes, so I assume these
whitespace-only changes didn't break anything (and shouldn't have).
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@360190 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Previously, I had added the ast_sched_thread stuff that was a generic scheduler
thread implementation. However, if you used it, it required using different
functions for modifying scheduler contents. This patch reworks how this is
done and just allows you to optionally start a thread on the original scheduler
context structure that has always been there. This makes it trivial to switch
to the generic scheduler thread implementation without having to touch any of
the other code that adds or removes scheduler entries.
In passing, I made some naming tweaks to add ast_ prefixes where they were not
there before.
Review: https://reviewboard.asterisk.org/r/1007/
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@299091 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/branches/1.8
................
r281575 | russell | 2010-08-10 13:05:07 -0500 (Tue, 10 Aug 2010) | 16 lines
Merged revisions 281574 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2
........
r281574 | russell | 2010-08-10 13:04:32 -0500 (Tue, 10 Aug 2010) | 9 lines
Don't move the time threshold for running scheduled events on every iteration.
Instead, only calculate the time threshold each time ast_sched_runq() is called.
(closes issue #17742)
Reported by: schmidts
Patches:
sched.c.patch uploaded by schmidts (license 1077)
........
................
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@281576 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This patch changes the scheduler to use a max-heap to store pending scheduler
entries instead of a fully sorted doubly linked list. When the number of
entries in the scheduler gets large, this will perform much better. For much
more detailed information on this change, see the review request.
Review: http://reviewboard.digium.com/r/160/
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@176639 65c4cc65-6c06-0410-ace0-fbb531ad65f3
1) It had numerous coding guidelines violations with regards to formatting.
2) It allocated memory using ast_calloc() that was never freed.
3) It didn't check for failure from the allocation.
4) It used sprintf() and strcat() to build the result, doing zero checking to
prevent writing past the end of the provided buffer.
The function also lacks API documentation, but that has not been addressed in
this commit.
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@175829 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit expands the Asterisk scheduler API to include a common implementation
of a scheduler context being processed by a dedicated thread. chan_iax2 has been
updated to use this new code. Also, as a result, this resolves some race
conditions related to the previous chan_iax2 scheduler handling.
Related to rev 171452 which resolved the same issues in 1.4.
Code from team/russell/sched_thread2
Review: http://reviewboard.digium.com/r/129/
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@173858 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r142354 | russell | 2008-09-10 11:39:53 -0500 (Wed, 10 Sep 2008) | 7 lines
It is a normal situation that a task gets put in the scheduler that should run
as soon as possible. Accept "0" as an acceptable time to run, and also treat
negative as "run now", and don't print a debug message about it.
(inspired by a message asking about the "request to schedule in the past"
debug message on the -dev list)
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@142355 65c4cc65-6c06-0410-ace0-fbb531ad65f3
of platform/compiler-dependent warnings when handing
struct timeval fields, both reading and printing them.
It is a lost battle to handle the different ways struct timeval
is handled on the various platforms and compilers, so try
to be pragmatic and go through int/long which are universally
supported.
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@116557 65c4cc65-6c06-0410-ace0-fbb531ad65f3
to use doubly linked lists. The schedule() function had an optimization that
had it try to guess which direction would be better for the traversal to insert
the task into the scheduler queue. However, if the code chose the path where
it traversed the queue in reverse, and the result was that the task should be
at the head of the queue, then the code would actually put it at the tail,
instead.
(Problem found by bbryant, debugged and fixed by bbryant and me)
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@115537 65c4cc65-6c06-0410-ace0-fbb531ad65f3
build times - tested, there is no measureable difference before and
after this commit.
In this change:
use asterisk/compat.h to include a small set of system headers:
inttypes.h, unistd.h, stddef.h, stddint.h, sys/types.h, stdarg.h,
stdlib.h, alloca.h, stdio.h
Where available, the inclusion is conditional on HAVE_FOO_H as determined
by autoconf.
Normally, source files should not include any of the above system headers,
and instead use either "asterisk.h" or "asterisk/compat.h" which does it
better.
For the time being I have left alone second-level directories
(main/db1-ast, etc.).
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@89333 65c4cc65-6c06-0410-ace0-fbb531ad65f3
- the *_CURRENT macros no longer need the list head pointer argument
- add AST_LIST_MOVE_CURRENT to encapsulate the remove/add operation when moving entries between lists
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@89106 65c4cc65-6c06-0410-ace0-fbb531ad65f3
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r83432 | russell | 2007-09-21 09:37:20 -0500 (Fri, 21 Sep 2007) | 4 lines
gcc 4.2 has a new set of warnings dealing with cosnt pointers. This set of
changes gets all of Asterisk (minus chan_alsa for now) to compile with gcc 4.2.
(closes issue #10774, patch from qwell)
........
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@83433 65c4cc65-6c06-0410-ace0-fbb531ad65f3
in place of a very common construct. I also used it in a number of places
in chan_sip.
if (id > -1)
ast_sched_del(sched, id);
id = ast_sched_add(sched, ...);
changes to:
ast_sched_replace(id, sched, ...);
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@79861 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Mike Bradeen
Walter Doekes
Sean Bright
Joshua Colp
Corey Farrell
Richard Mudgett
Mark Michelson
Yousf Ateya
Matt Jordan
Jonathan Rose
Kinsey Moore
Kevin P. Fleming
Russell Bryant
Tilghman Lesher
Sean Bright
Luigi Rizzo
Steve Murphy