[svn-commits] murf: branch murf/bug11210 r104064 - in /team/murf/bug11210: channels/ includ...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Sat Feb 23 13:16:27 CST 2008
Author: murf
Date: Sat Feb 23 13:16:26 2008
New Revision: 104064
URL: http://svn.digium.com/view/asterisk?view=rev&rev=104064
Log:
Checkpoint: Modifications as per Russell's review of the code. An extra find routine added to the sched interface, just in case.
Modified:
team/murf/bug11210/channels/chan_sip.c
team/murf/bug11210/include/asterisk/sched.h
team/murf/bug11210/main/sched.c
Modified: team/murf/bug11210/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/channels/chan_sip.c?view=diff&rev=104064&r1=104063&r2=104064
==============================================================================
--- team/murf/bug11210/channels/chan_sip.c (original)
+++ team/murf/bug11210/channels/chan_sip.c Sat Feb 23 13:16:26 2008
@@ -1411,7 +1411,7 @@
/* things that don't belong in flags */
char is_realtime; /*!< this is a 'realtime' user */
- char the_mark; /*!< moved out of the ASTOBJ fields; that which bears the_mark should be deleted! */
+ unsigned int the_mark:1; /*!< moved out of the ASTOBJ fields; that which bears the_mark should be deleted! */
int amaflags; /*!< AMA flags for billing */
int callingpres; /*!< Calling id presentation */
@@ -2453,7 +2453,6 @@
AST_SCHED_DEL_UNREF(sched, dialog->autokillid, dialog_unref(dialog, "when you delete the autokillid sched, you should dec the refcount for the stored dialog ptr"));
dialog_unref(dialog, "Let's unbump the count in the unlink so the poor pvt can disappear if it is time");
- /* dialog_unref(dialog, "One final decrement to counter the creation of the dialog and allow its destruction"); this is just plain wrong. any scenario that requires this is just WRONG. */
return NULL;
}
@@ -2704,7 +2703,7 @@
int res = 0;
const struct sockaddr_in *dst = sip_real_dst(p);
- /* ast_log(LOG_DEBUG, "Trying to put '%.10s' onto %s socket...\n", data, get_transport(p->socket.type)); */
+ ast_debug(3, "Trying to put '%.10s' onto %s socket...\n", data, get_transport(p->socket.type));
if (sip_prepare_socket(p) < 0)
return XMIT_ERROR;
@@ -3063,17 +3062,8 @@
if (p->relatedpeer)
p->relatedpeer = unref_peer(p->relatedpeer, "__sip_autodestruct: unref peer p->relatedpeer"); /* Remove link to peer. If it's realtime, make sure it's gone from memory) */
- /* sometimes, the unref from deleting the autokillid sched item, will
- free the dialog; we need to hang on a little longer and make sure it's cleaned out first */
- dialog_ref(p, "Bump counter in autodestruct to prevent premature destructio of dialog");
/* Reset schedule ID */
- if (p->autokillid != -1) {
- /* a SCHED_DEL(_UNREF) was done here... which makes no sense... it's ALWAYS removed from the queue
- before the callback is called... */
-
- p->autokillid = -1; /* but, it is a really good idea to unset the autokillid field if its truly bogus now */
- append_history(p, "CancelDestroy", "");
- }
+ p->autokillid = -1;
if (p->owner) {
ast_log(LOG_WARNING, "Autodestruct on dialog '%s' with owner in place (Method: %s)\n", p->callid, sip_methods[p->method].text);
@@ -3092,7 +3082,6 @@
/* sip_destroy also absorbs the reference */
}
dialog_unref(p, "The ref to a dialog passed to this sched callback is going out of scope; unref it.");
- dialog_unref(p, "UNBump counter in autodestruct to ALLOW destructio of dialog");
return 0;
}
@@ -3677,8 +3666,8 @@
*
* NOTE: once peer is refcounted, this probably is no longer necessary.
*/
- AST_SCHED_DEL(sched, peer->expire); /* HUH? At some future time, if we need to refcount peers.... */
- AST_SCHED_DEL(sched, peer->pokeexpire); /* HUH? At some future time, if we need to refcount peers.... */
+ AST_SCHED_DEL(sched, peer->expire);
+ AST_SCHED_DEL(sched, peer->pokeexpire);
register_peer_exten(peer, FALSE);
ast_free_ha(peer->ha);
@@ -3903,9 +3892,9 @@
/* Cache peer */
ast_copy_flags(&peer->flags[1], &global_flags[1], SIP_PAGE2_RTAUTOCLEAR|SIP_PAGE2_RTCACHEFRIENDS);
if (ast_test_flag(&global_flags[1], SIP_PAGE2_RTAUTOCLEAR)) {
- AST_SCHED_REPLACE(peer->expire, sched, global_rtautoclear * 1000, expire_register, (void *) peer); /* HUH? peer is a refcounted object, and we are storing it in the sched struct, so
- really, really, we should be incr. its refcount right here, but I guess, since
- peers hang around until module unload time anyway, it's not worth the trouble */
+ AST_SCHED_REPLACE(peer->expire, sched, global_rtautoclear * 1000, expire_register, (void *) peer);
+ /* we could be incr. its refcount right here, but I guess, since
+ peers hang around until module unload time anyway, it's not worth the trouble */
}
ao2_t_link(peers, peer, "link peer into peers table");
if (peer->addr.sin_addr.s_addr) {
@@ -3925,6 +3914,12 @@
return peer;
}
+/*! \brief Locate peer by name or ip address
+ * This is used on incoming SIP message to find matching peer on ip
+ or outgoing message to find matching peer on name
+ \note Avoid using this function in new functions if there is a way to avoid it, i
+ since it might cause a database lookup.
+*/
static struct sip_peer *find_peer(const char *peer, struct sockaddr_in *sin, int realtime)
{
struct sip_peer *p = NULL;
@@ -4021,7 +4016,12 @@
/*! \brief Locate user by name
* Locates user by name (From: sip uri user name part) first
* from in-memory list (static configuration) then from
- * realtime storage (defined in extconfig.conf) */
+ * realtime storage (defined in extconfig.conf)
+ * \return a refcounted pointer to a user. Make sure the
+ * the ref count is decremented when this pointer is nulled, changed,
+ * or goes out of scope!
+ */
+
static struct sip_user *find_user(const char *name, int realtime)
{
struct sip_user tmp;
@@ -4410,7 +4410,9 @@
p->invitestate = INV_CALLING;
/* Initialize auto-congest time */
- AST_SCHED_REPLACE(p->initid, sched, p->timer_b, auto_congest, p /* replace should not affect the refcount */);
+ AST_SCHED_REPLACE_UNREF(p->initid, sched, p->timer_b, auto_congest, p,
+ dialog_unref(p, "object ptr dec when SCHED_REPLACE del op succeeded or add failed"),
+ dialog_ref(p, "object ptr inc when SCHED_REPLACE add succeeded") );
}
return res;
}
@@ -4431,11 +4433,8 @@
reg->call = dialog_unref(reg->call, "unref reg->call");
/* reg->call = sip_destroy(reg->call); */
}
- AST_SCHED_DEL(sched, reg->expire); /* HUH? normally, if reg were being refcounted thru sched calls, we'd
- add code after this to unref the reg */
-
- AST_SCHED_DEL(sched, reg->timeout); /* HUH? normally, if reg were being refcounted thru sched calls, we'd
- add code after this to unref the reg */
+ AST_SCHED_DEL(sched, reg->expire);
+ AST_SCHED_DEL(sched, reg->timeout);
ast_string_field_free_memory(reg);
ast_atomic_fetchadd_int(®objs, -1);
@@ -9246,7 +9245,7 @@
static int sip_reregister(const void *data)
{
/* if we are here, we know that we need to reregister. */
- struct sip_registry *r= registry_addref((struct sip_registry *) data, "reg_addref from sip_reregister");
+ struct sip_registry *r= (struct sip_registry *) data;
/* if we couldn't get a reference to the registry object, punt */
if (!r)
@@ -9284,7 +9283,7 @@
{
/* if we are here, our registration timed out, so we'll just do it over */
- struct sip_registry *r = registry_addref((struct sip_registry *) data, "registry_addref from sip_reg_timeout");
+ struct sip_registry *r = (struct sip_registry *)data; /* the ref count should have been bumped when the sched item was added */
struct sip_pvt *p;
int res;
@@ -9380,10 +9379,12 @@
dialog_unlink_all(p, TRUE, TRUE);
p = dialog_unref(p, "unref dialog after unlink_all"); /* HUH? this might be bad! */
if (r->timeout > -1) {
- AST_SCHED_REPLACE(r->timeout, sched, global_reg_timeout * 1000, sip_reg_timeout, r);
+ AST_SCHED_REPLACE_UNREF(r->timeout, sched, global_reg_timeout * 1000, sip_reg_timeout, r,
+ registry_unref(r,"del for REPLACE of registry ptr"),
+ registry_addref(r,"add for REPLACE registry ptr"));
ast_log(LOG_WARNING, "Still have a registration timeout for %s@%s (create_addr() error), %d\n", r->username, r->hostname, r->timeout);
} else {
- r->timeout = ast_sched_add(sched, global_reg_timeout * 1000, sip_reg_timeout, r);
+ r->timeout = ast_sched_add(sched, global_reg_timeout * 1000, sip_reg_timeout, registry_addref(r, "add for REPLACE registry ptr"));
ast_log(LOG_WARNING, "Probably a DNS error for registration to %s@%s, trying REGISTER again (after %d seconds)\n", r->username, r->hostname, global_reg_timeout);
}
r->regattempts++;
@@ -9435,7 +9436,9 @@
if (auth == NULL) {
if (r->timeout > -1)
ast_log(LOG_WARNING, "Still have a registration timeout, #%d - deleting it\n", r->timeout);
- AST_SCHED_REPLACE(r->timeout, sched, global_reg_timeout * 1000, sip_reg_timeout, r);
+ AST_SCHED_REPLACE_UNREF(r->timeout, sched, global_reg_timeout * 1000, sip_reg_timeout, r,
+ registry_unref(r,"reg ptr unrefed from del in SCHED_REPLACE (or add failure)"),
+ registry_addref(r,"reg ptr reffed from add in SCHED_REPLACE"));
ast_debug(1, "Scheduled a registration timeout for %s id #%d \n", r->hostname, r->timeout);
}
@@ -10851,7 +10854,7 @@
- Their tag is fromtag, our tag is to-tag
- This means that in some transactions, totag needs to be their tag :-)
depending upon the direction
- Returns a reference, remember to release it when done XXX
+ \return a reference, remember to release it when done
*/
static struct sip_pvt *get_sip_pvt_byid_locked(const char *callid, const char *totag, const char *fromtag)
{
@@ -12031,6 +12034,7 @@
count++;
if (havepattern && regexec(®exbuf, user->name, 0, NULL, 0)) {
ao2_unlock(user);
+ unref_user(user, "toss iterator pointer via a continue in iterator loop");
continue;
}
@@ -13013,24 +13017,24 @@
static char *sip_show_sched(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
{
char cbuf[2256];
- struct ast_cb_names cbnames = {9, { "retrans_pkt",
- "__sip_autodestruct",
- "expire_register",
- "auto_congest",
- "sip_reg_timeout",
- "sip_poke_peer_s",
- "sip_poke_noanswer",
- "sip_reregister",
- "sip_reinvite_retry"},
+ struct ast_cb_names cbnames = {9, { "retrans_pkt",
+ "__sip_autodestruct",
+ "expire_register",
+ "auto_congest",
+ "sip_reg_timeout",
+ "sip_poke_peer_s",
+ "sip_poke_noanswer",
+ "sip_reregister",
+ "sip_reinvite_retry"},
{ retrans_pkt,
- __sip_autodestruct,
- expire_register,
- auto_congest,
- sip_reg_timeout,
- sip_poke_peer_s,
- sip_poke_noanswer,
- sip_reregister,
- sip_reinvite_retry}};
+ __sip_autodestruct,
+ expire_register,
+ auto_congest,
+ sip_reg_timeout,
+ sip_poke_peer_s,
+ sip_poke_noanswer,
+ sip_reregister,
+ sip_reinvite_retry}};
switch (cmd) {
case CLI_INIT:
@@ -13148,7 +13152,7 @@
ast_cli(a->fd, FORMAT2, "Host", "Username", "Refresh", "State", "Reg.Time");
ASTOBJ_CONTAINER_TRAVERSE(®l, 1, do {
- ASTOBJ_RDLOCK(iterator); /* was RDLOCK */
+ ASTOBJ_RDLOCK(iterator);
snprintf(host, sizeof(host), "%s:%d", iterator->hostname, iterator->portno ? iterator->portno : STANDARD_SIP_PORT);
if (iterator->regtime.tv_sec) {
ast_localtime(&iterator->regtime, &tm, NULL);
@@ -13439,7 +13443,7 @@
if (cur->subscribed == NONE && !arg->subscriptions) {
/* set if SIP transfer in progress */
const char *referstatus = cur->refer ? referstatus2str(cur->refer->status) : "";
- char formatbuf[BUFSIZ/2];
+ char formatbuf[SIPBUFSIZE/2];
ast_cli(arg->fd, FORMAT, ast_inet_ntoa(dst->sin_addr),
S_OR(cur->username, S_OR(cur->cid_num, "(None)")),
@@ -13534,11 +13538,11 @@
if (!strncasecmp(word, cur->callid, wordlen) && ++which > state) {
c = ast_strdup(cur->callid);
sip_pvt_unlock(cur);
- ao2_t_ref(cur, -1, "drop ref in iterator loop break");
+ dialog_unref(cur, "drop ref in iterator loop break");
break;
}
sip_pvt_unlock(cur);
- ao2_t_ref(cur, -1, "drop ref in iterator loop");
+ dialog_unref(cur, "drop ref in iterator loop");
}
return c;
}
@@ -13559,11 +13563,10 @@
(!flags2 || ast_test_flag(&peer->flags[1], flags2)) &&
++which > state)
result = ast_strdup(peer->name);
+ unref_peer(peer, "toss iterator peer ptr before break");
if (result) {
- unref_peer(peer, "toss iterator peer ptr before break");
break;
}
- unref_peer(peer, "toss iterator peer ptr");
}
return result;
}
@@ -14853,8 +14856,8 @@
struct sip_pvt *p = (struct sip_pvt *) data;
ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);
+ p->waitid = -1;
dialog_unref(p, "unref the dialog ptr from sip_reinvite_retry, because it held a dialog ptr");
- p->waitid = -1;
return 0;
}
@@ -15278,7 +15281,7 @@
{
int expires, expires_ms;
struct sip_registry *r;
- r=p->registry; /* do I need to ref/unref this during this routine? Doesn't LOOK like it...*/
+ r=p->registry;
switch (resp) {
case 401: /* Unauthorized */
@@ -15412,14 +15415,12 @@
r->refresh= (int) expires_ms / 1000;
/* Schedule re-registration before we expire */
- AST_SCHED_REPLACE(r->expire, sched, expires_ms, sip_reregister, r);
+ AST_SCHED_REPLACE_UNREF(r->expire, sched, expires_ms, sip_reregister, r,
+ registry_unref(r,"unref in REPLACE del/add fail"),
+ registry_addref(r,"The Addition side of REPLACE"));
/* it is clear that we would not want to destroy the registry entry if we just
scheduled a callback and recorded it in there! */
/* since we never bumped the count, we shouldn't decrement it! registry_unref(r, "unref registry ptr r");*/ /* HUH? if this gets deleted, p->registry will be a bad pointer! */
- /* shouldn't this be:
- p->registry = registry_unref(r, "unref registry entry at p->registry");
- ???????
- */
}
return 1;
}
@@ -15427,7 +15428,7 @@
/*! \brief Handle qualification responses (OPTIONS) */
static void handle_response_peerpoke(struct sip_pvt *p, int resp, struct sip_request *req)
{
- struct sip_peer *peer = ref_peer(p->relatedpeer, "bump refcount on p, as it is being used in this function(handle_response_peerpoke)"); /* hope this is already refcounted! */
+ struct sip_peer *peer = /* ref_peer( */ p->relatedpeer /* , "bump refcount on p, as it is being used in this function(handle_response_peerpoke)")*/ ; /* hope this is already refcounted! */
int statechanged, is_reachable, was_reachable;
int pingtime = ast_tvdiff_ms(ast_tvnow(), peer->ps);
@@ -15469,7 +15470,7 @@
AST_SCHED_REPLACE(peer->pokeexpire, sched,
is_reachable ? peer->qualifyfreq : DEFAULT_FREQ_NOTOK,
sip_poke_peer_s, peer);
- unref_peer(peer, "unref relatedpeer ptr var at end of handle_response_peerpoke");
+ /* unref_peer(peer, "unref relatedpeer ptr var at end of handle_response_peerpoke"); */
}
/*! \brief Immediately stop RTP, VRTP and UDPTL as applicable */
@@ -18055,8 +18056,7 @@
/* we need to dec the refcount, now that the extensionstate is removed */
dialog_unref(p, "the extensionstate containing this dialog ptr was deleted");
}
- p->stateid = ast_extension_state_add(p->context, p->exten, cb_extensionstate, p); /* Here we go -- saving another dialog ptr in an external struct */
- dialog_ref(p, "stored a dialog ptr in an extensionstate struct");
+ p->stateid = ast_extension_state_add(p->context, p->exten, cb_extensionstate, dialog_ref(p,"copying dialog ptr into extension state struct"));
}
if (!req->ignore && p)
@@ -18721,7 +18721,7 @@
return;
/* If the call is not in UP state or redirected outside Asterisk, no need to check timers */
- if (dialog->owner->_state != AST_STATE_UP || dialog->redirip.sin_addr.s_addr) /* CRASH HERE because dialog->owner is NULL */
+ if (dialog->owner->_state != AST_STATE_UP || dialog->redirip.sin_addr.s_addr)
return;
/* If the call is involved in a T38 fax session do not check RTP timeout */
@@ -19241,7 +19241,6 @@
{
struct sip_pvt *p;
int xmitres = 0;
- char mbuf3[200];
if (!peer->maxms || !peer->addr.sin_addr.s_addr) {
/* IF we have no IP, or this isn't to be monitored, return
@@ -19262,8 +19261,7 @@
}
if (!(p = sip_alloc(NULL, NULL, 0, SIP_OPTIONS)))
return -1;
- sprintf(mbuf3,"copy sip alloc from p to peer->call (peer is %p [%s])", peer, peer->name);
- peer->call = dialog_ref(p, mbuf3);
+ peer->call = dialog_ref(p, "copy sip alloc from p to peer->call");
p->sa = peer->addr;
p->recv = peer->addr;
@@ -21054,7 +21052,7 @@
is this registry entry. In other words, if the dialog we are pointing to points back to
us, then if we get a lock on this object, and try to UNREF it, we will deadlock, because
we already ... NO. This is not the problem. */
- ASTOBJ_RDLOCK(iterator); /* was RDLOCK and now regl is locked, and the object is also locked */
+ ASTOBJ_RDLOCK(iterator); /* now regl is locked, and the object is also locked */
if (iterator->call) {
ast_debug(3, "Destroying active SIP dialog for registry %s@%s\n", iterator->username, iterator->hostname);
/* This will also remove references to the registry */
@@ -22254,10 +22252,11 @@
regspacing = 100;
ms = regspacing;
ASTOBJ_CONTAINER_TRAVERSE(®l, 1, do {
- ASTOBJ_WRLOCK(iterator); /* was WRLOCK */
+ ASTOBJ_WRLOCK(iterator);
ms += regspacing;
- AST_SCHED_REPLACE(iterator->expire,
- sched, ms, sip_reregister, iterator);
+ AST_SCHED_REPLACE_UNREF(iterator->expire, sched, ms, sip_reregister, iterator,
+ registry_unref(iterator, "REPLACE sched del decs the refcount"),
+ registry_unref(iterator, "REPLACE sched add incs the refcount"));
ASTOBJ_UNLOCK(iterator);
} while (0)
);
Modified: team/murf/bug11210/include/asterisk/sched.h
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/include/asterisk/sched.h?view=diff&rev=104064&r1=104063&r2=104064
==============================================================================
--- team/murf/bug11210/include/asterisk/sched.h (original)
+++ team/murf/bug11210/include/asterisk/sched.h Sat Feb 23 13:16:26 2008
@@ -70,6 +70,25 @@
#define AST_SCHED_REPLACE(id, sched, when, callback, data) \
AST_SCHED_REPLACE_VARIABLE(id, sched, when, callback, data, 0)
+
+#define AST_SCHED_REPLACE_VARIABLE_UNREF(id, sched, when, callback, data, variable, unrefcall, refcall) \
+ do { \
+ int _count = 0, _res=1; \
+ while (id > -1 && (_res = ast_sched_del(sched, id) && _count++ < 10)) \
+ usleep(1); \
+ if (!_res) \
+ unrefcall; \
+ if (_count == 10) \
+ ast_log(LOG_WARNING, "Unable to cancel schedule ID %d. This is probably a bug (%s: %s, line %d).\n", id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \
+ id = ast_sched_add_variable(sched, when, callback, data, variable); \
+ if (id == -1) \
+ unrefcall; \
+ else \
+ refcall; \
+ } while (0);
+
+#define AST_SCHED_REPLACE_UNREF(id, sched, when, callback, data, unrefcall, refcall) \
+ AST_SCHED_REPLACE_VARIABLE_UNREF(id, sched, when, callback, data, 0, unrefcall, refcall)
struct sched_context;
@@ -156,6 +175,15 @@
*/
int ast_sched_replace_variable(int old_id, struct sched_context *con, int when, ast_sched_cb callback, const void *data, int variable);
+
+/*! \brief Find a sched structure and return the data field associated with it.
+ * \param con scheduling context in which to search fro the matching id
+ * \param id ID of the scheduled item to find
+ * \return the data field from the matching sched struct if found; else return NULL if not found.
+ */
+
+const void *ast_sched_find_data(struct sched_context *con, int id);
+
/*! \brief Deletes a scheduled event
* Remove this event from being run. A procedure should not remove its own
* event, but return 0 instead. In most cases, you should not call this
@@ -165,6 +193,7 @@
* \param id ID of the scheduled item to delete
* \return Returns 0 on success, -1 on failure
*/
+
int ast_sched_del(struct sched_context *con, int id);
/*! \brief Determines number of seconds until the next outstanding event to take place
Modified: team/murf/bug11210/main/sched.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/main/sched.c?view=diff&rev=104064&r1=104063&r2=104064
==============================================================================
--- team/murf/bug11210/main/sched.c (original)
+++ team/murf/bug11210/main/sched.c Sat Feb 23 13:16:26 2008
@@ -305,6 +305,16 @@
return ast_sched_add_variable(con, when, callback, data, 0);
}
+const void *ast_sched_find_data(struct sched_context *con, int id)
+{
+ struct sched tmp,*res;
+ tmp.id = id;
+ res = ast_hashtab_lookup(con->schedq_ht, &tmp);
+ if (res)
+ return res->data;
+ return NULL;
+}
+
/*! \brief
* Delete the schedule entry with number
* "id". It's nearly impossible that there
More information about the svn-commits
mailing list