From 3eeec9a7ec11aee0cbecf56bf721ffb7e4b81bf9 Mon Sep 17 00:00:00 2001 From: Henning Westerholt Date: Tue, 30 Jul 2019 13:06:48 +0200 Subject: [PATCH] tm: remove old timer based transaction delete functionality, not active since 2007 - remove old timer based transaction delete functionality - the current implementation is active since 2007 (commit e67d950955e5dc3d) - remove related TM_DEL_UNREF #defines and #ifdef --- src/modules/tm/h_table.c | 4 --- src/modules/tm/h_table.h | 16 ----------- src/modules/tm/t_cancel.c | 5 ---- src/modules/tm/t_funcs.h | 14 --------- src/modules/tm/t_lookup.c | 5 ---- src/modules/tm/t_stats.c | 4 --- src/modules/tm/timer.c | 60 --------------------------------------- src/modules/tm/uac.c | 6 ---- 8 files changed, 114 deletions(-) diff --git a/src/modules/tm/h_table.c b/src/modules/tm/h_table.c index b54eb3a136c..dbba066c690 100644 --- a/src/modules/tm/h_table.c +++ b/src/modules/tm/h_table.c @@ -590,11 +590,7 @@ void tm_log_transaction(tm_cell_t *tcell, int llev, char *ltext) (tcell->uas.request)?"yes":"no", (unsigned)tcell->flags, (unsigned)tcell->nr_of_outgoings, -#ifdef TM_DEL_UNREF (unsigned)atomic_get(&tcell->ref_count), -#else - tcell->ref_count, -#endif (unsigned)TICKS_TO_S(tcell->end_of_life) ); diff --git a/src/modules/tm/h_table.h b/src/modules/tm/h_table.h index ffd230f7716..5486c1c4f04 100644 --- a/src/modules/tm/h_table.h +++ b/src/modules/tm/h_table.h @@ -31,7 +31,6 @@ #include "defs.h" #include "t_stats.h" -#define TM_DEL_UNREF /* uncomment the next define if you wish to keep hash statistics*/ /* #define TM_HASH_STATS @@ -365,8 +364,6 @@ typedef struct cell /* free operations counter - debug */ int fcount; - -#ifdef TM_DEL_UNREF /* every time the transaction/cell is referenced from somewhere this * ref_count should be increased (via REF()) and every time the reference * is removed the ref_count should be decreased (via UNREF()). @@ -378,19 +375,6 @@ typedef struct cell * it will be automatically deleted by the UNREF() operation. */ atomic_t ref_count; -#else - /* how many processes are currently processing this transaction ; - * note that only processes working on a request/reply belonging - * to a transaction increase ref_count -- timers don't, since we - * rely on transaction state machine to clean-up all but wait timer - * when entering WAIT state and the wait timer is the only place - * from which a transaction can be deleted (if ref_count==0); good - * for protecting from conditions in which wait_timer hits and - * tries to delete a transaction whereas at the same time - * a delayed message belonging to the transaction is received */ - volatile unsigned int ref_count; -#endif - /* needed for generating local ACK/CANCEL for local * transactions; all but cseq_n include the entire * header field value, cseq_n only Cseq number; with diff --git a/src/modules/tm/t_cancel.c b/src/modules/tm/t_cancel.c index 3a6da2d87ea..663fa8f8e26 100644 --- a/src/modules/tm/t_cancel.c +++ b/src/modules/tm/t_cancel.c @@ -153,11 +153,6 @@ int cancel_all_uacs(struct cell *trans, int how) i=cancel_uacs(trans, &cancel_data, how); if (how & F_CANCEL_UNREF) -#ifndef TM_DEL_UNREF - /* in case of 'too many' _buggy_ invocations, the ref count (a uint) might - * actually wrap around, possibly leaving the T leaking. */ -#warning "use of F_CANCEL_UNREF flag is unsafe without defining TM_DEL_UNREF" -#endif UNREF(trans); /* count the still active branches */ diff --git a/src/modules/tm/t_funcs.h b/src/modules/tm/t_funcs.h index 1b90c20d453..6de42465d9c 100644 --- a/src/modules/tm/t_funcs.h +++ b/src/modules/tm/t_funcs.h @@ -94,8 +94,6 @@ int send_pr_buffer( struct retr_buf *rb, void *buf, int len); -#ifdef TM_DEL_UNREF - #define UNREF_FREE(_T_cell, _T_unlinked) \ do{\ if (atomic_dec_and_test(&(_T_cell)->ref_count)){ \ @@ -128,18 +126,6 @@ int send_pr_buffer( struct retr_buf *rb, void *buf, int len); #define REF_UNSAFE(_T_cell) REF(_T_cell) #define INIT_REF(_T_cell, v) atomic_set(&(_T_cell)->ref_count, v) -#else - -#define UNREF_UNSAFE(_T_cell) ((_T_cell)->ref_count--) -#define UNREF(_T_cell) do{ \ - LOCK_HASH( (_T_cell)->hash_index ); \ - UNREF_UNSAFE(_T_cell); \ - UNLOCK_HASH( (_T_cell)->hash_index ); }while(0) -#define REF_UNSAFE(_T_cell) ((_T_cell)->ref_count++) -#define INIT_REF_UNSAFE(_T_cell) ((_T_cell)->ref_count=1) -#define IS_REFFED_UNSAFE(_T_cell) ((_T_cell)->ref_count!=0) - -#endif /* * Parse and fixup the fr_*_timer AVP specs */ diff --git a/src/modules/tm/t_lookup.c b/src/modules/tm/t_lookup.c index 45cbf0b4ed4..08a669f0d0b 100644 --- a/src/modules/tm/t_lookup.c +++ b/src/modules/tm/t_lookup.c @@ -1305,15 +1305,10 @@ static inline int new_t(struct sip_msg *p_msg) return E_OUT_OF_MEM; } -#ifdef TM_DEL_UNREF INIT_REF(new_cell, 2); /* 1 because it will be ref'ed from the * hash and +1 because we set T to it */ -#endif insert_into_hash_table_unsafe( new_cell, p_msg->hash_index ); set_t(new_cell, T_BR_UNDEFINED); -#ifndef TM_DEL_UNREF - INIT_REF_UNSAFE(T); -#endif /* init pointers to headers needed to construct local * requests such as CANCEL/ACK */ diff --git a/src/modules/tm/t_stats.c b/src/modules/tm/t_stats.c index 5917b46a886..6e6228b476d 100644 --- a/src/modules/tm/t_stats.c +++ b/src/modules/tm/t_stats.c @@ -286,11 +286,7 @@ void tm_rpc_list(rpc_t* rpc, void* c) "uas_request", (tcell->uas.request)?"yes":"no", "tflags", (unsigned)tcell->flags, "outgoings", (unsigned)tcell->nr_of_outgoings, -#ifdef TM_DEL_UNREF "ref_count", (unsigned)atomic_get(&tcell->ref_count), -#else - "ref_count", tcell->ref_count, -#endif "lifetime", (unsigned)TICKS_TO_S(tcell->end_of_life) ); } diff --git a/src/modules/tm/timer.c b/src/modules/tm/timer.c index 9f8529ab715..5ab91e6a06f 100644 --- a/src/modules/tm/timer.c +++ b/src/modules/tm/timer.c @@ -286,39 +286,6 @@ int timer_fixup_ms(void *handle, str *gname, str *name, void **val) /******************** handlers ***************************/ -#ifndef TM_DEL_UNREF -/* returns number of ticks before retrying the del, or 0 if the del. - * was succesfull */ -inline static ticks_t delete_cell(struct cell *p_cell, int unlock) -{ - /* there may still be FR/RETR timers, which have been reset - (i.e., time_out==TIMER_DELETED) but are stilled linked to - timer lists and must be removed from there before the - structures are released - */ - unlink_timers(p_cell); - /* still in use ... don't delete */ - if(IS_REFFED_UNSAFE(p_cell)) { - if(unlock) - UNLOCK_HASH(p_cell->hash_index); - LM_DBG("%p: can't delete -- still reffed (%d)\n", p_cell, - p_cell->ref_count); - /* delay the delete */ - /* TODO: change refcnts and delete on refcnt==0 */ - return cfg_get(tm, tm_cfg, delete_timeout); - } else { - if(unlock) - UNLOCK_HASH(p_cell->hash_index); -#ifdef EXTRA_DEBUG - LM_DBG("delete transaction %p\n", p_cell); -#endif - free_cell(p_cell); - return 0; - } -} -#endif /* TM_DEL_UNREF */ - - /* generate a fake reply * it assumes the REPLY_LOCK is already held and returns unlocked */ static void fake_reply(struct cell *t, int branch, int code) @@ -640,8 +607,6 @@ ticks_t wait_handler(ticks_t ti, struct timer_ln *wait_tl, void *data) #ifdef TIMER_DEBUG LM_DBG("WAIT timer hit @%d for %p (timer_lm %p)\n", ti, p_cell, wait_tl); #endif - -#ifdef TM_DEL_UNREF /* stop cancel timers if any running */ if(is_invite(p_cell)) { cleanup_localcancel_timers(p_cell); @@ -688,30 +653,5 @@ ticks_t wait_handler(ticks_t ti, struct timer_ln *wait_tl, void *data) p_cell->flags |= T_IN_AGONY; UNREF_FREE(p_cell, unlinked); ret = 0; -#else /* TM_DEL_UNREF */ - if(p_cell->flags & T_IN_AGONY) { - /* delayed delete */ - /* we call delete now without any locking on hash/ref_count; - we can do that because delete_handler is only entered after - the delete timer was installed from wait_handler, which - removed transaction from hash table and did not destroy it - because some processes were using it; that means that the - processes currently using the transaction can unref and no - new processes can ref -- we can wait until ref_count is - zero safely without locking - */ - ret = delete_cell(p_cell, 0 /* don't unlock on return */); - } else { - /* stop cancel timers if any running */ - if(is_invite(p_cell)) - cleanup_localcancel_timers(p_cell); - /* remove the cell from the hash table */ - LOCK_HASH(p_cell->hash_index); - remove_from_hash_table_unsafe(p_cell); - p_cell->flags |= T_IN_AGONY; - /* delete (returns with UNLOCK-ed_HASH) */ - ret = delete_cell(p_cell, 1 /* unlock on return */); - } -#endif /* TM_DEL_UNREF */ return ret; } diff --git a/src/modules/tm/uac.c b/src/modules/tm/uac.c index 340c011d6e2..efe592846c3 100644 --- a/src/modules/tm/uac.c +++ b/src/modules/tm/uac.c @@ -494,9 +494,7 @@ static inline int t_uac_prepare(uac_req_t *uac_r, #endif if (!is_ack) { -#ifdef TM_DEL_UNREF INIT_REF(new_cell, 1); /* ref'ed only from the hash */ -#endif hi=dlg2hash(uac_r->dialog); LOCK_HASH(hi); insert_into_hash_table_unsafe(new_cell, hi); @@ -585,7 +583,6 @@ static inline int t_uac_prepare(uac_req_t *uac_r, } error2: -#ifdef TM_DEL_UNREF if (is_ack) { free_cell(new_cell); } else { @@ -596,9 +593,6 @@ static inline int t_uac_prepare(uac_req_t *uac_r, UNREF_FREE(new_cell, 0); } } -#else - free_cell(new_cell); -#endif error3: return ret; }