Ticket #1004: Symbian timer heap fix

 - timer heap now records active timers and cancel them when it's destroyed


git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@3034 74dad513-b988-da41-8d7b-12977e46ad98

2 files changed, 177 insertions, 34 deletions

diff --git a/pjlib/include/pj/timer.h b/pjlib/include/pj/timer.h
index dbcae718..24d2bcb5 100644
--- a/pjlib/include/pj/timer.h
+++ b/pjlib/include/pj/timer.h
@@ -64,11 +64,7 @@ PJ_BEGIN_DECL
 /**
  * The type for internal timer ID.
  */
-#if defined(PJ_SYMBIAN) && PJ_SYMBIAN!=0
-typedef void *pj_timer_id_t;
-#else
 typedef int pj_timer_id_t;
-#endif
 
 /** 
  * Forward declaration for pj_timer_entry. 
diff --git a/pjlib/src/pj/timer_symbian.cpp b/pjlib/src/pj/timer_symbian.cpp
index a1a9052f..8f268804 100644
--- a/pjlib/src/pj/timer_symbian.cpp
+++ b/pjlib/src/pj/timer_symbian.cpp
@@ -31,6 +31,16 @@
 // Maximum number of miliseconds that RTimer.At() supports
 #define MAX_RTIMER_INTERVAL		2147
 
+/* Absolute maximum number of timer entries */
+#ifndef PJ_SYMBIAN_TIMER_MAX_COUNT
+#  define PJ_SYMBIAN_TIMER_MAX_COUNT	65535
+#endif
+
+/* Get the number of free slots in the timer heap */
+#define FREECNT(th)	(th->max_size - th->cur_size)
+
+// Forward declaration
+class CPjTimerEntry;
 
 /**
  * The implementation of timer heap.
@@ -43,18 +53,23 @@ struct pj_timer_heap_t
     /** Current size of the heap. */
     pj_size_t cur_size;
 
-    /** Max timed out entries to process per poll. */
-    unsigned max_entries_per_poll;
+    /** Array of timer entries. A scheduled timer will occupy one slot, and
+     *  the slot number will be saved in entry->_timer_id
+     */
+    CPjTimerEntry **entries;
+    
+    /** Array of free slot indexes in the "entries" array */
+    int *free_slots;
 };
 
-
-//////////////////////////////////////////////////////////////////////////////
 /**
  * Active object for each timer entry.
  */
 class CPjTimerEntry : public CActive 
 {
 public:
+    pj_timer_entry  *entry_;
+    
     static CPjTimerEntry* NewL(	pj_timer_heap_t *timer_heap,
     				pj_timer_entry *entry,
     				const pj_time_val *delay);
@@ -66,7 +81,6 @@ public:
 
 private:	
     pj_timer_heap_t *timer_heap_;
-    pj_timer_entry  *entry_;
     RTimer	     rtimer_;
     pj_uint32_t	     interval_left_;
     
@@ -75,10 +89,111 @@ private:
     void Schedule();
 };
 
+//////////////////////////////////////////////////////////////////////////////
+/*
+ * Implementation.
+ */
+
+/* Grow timer heap to the specified size */
+static pj_status_t realloc_timer_heap(pj_timer_heap_t *th, pj_size_t new_size)
+{
+    typedef CPjTimerEntry *entry_ptr;
+    CPjTimerEntry **entries = NULL;
+    int *free_slots = NULL;
+    unsigned i, j;
+ 
+    if (new_size > PJ_SYMBIAN_TIMER_MAX_COUNT) {
+	/* Just some sanity limit */
+	new_size = PJ_SYMBIAN_TIMER_MAX_COUNT;
+	if (new_size <= th->max_size) {
+	    /* We've grown large enough */
+	    pj_assert(!"Too many timer heap entries");
+	    return PJ_ETOOMANY;
+	}
+    }
+    
+    /* Allocate entries, move entries from the old array if there is one */
+    entries = new entry_ptr[new_size];
+    if (th->entries) {
+	pj_memcpy(entries, th->entries, th->max_size * sizeof(th->entries[0]));
+    }
+    /* Initialize the remaining new area */
+    pj_bzero(&entries[th->max_size], 
+	    (new_size - th->max_size) * sizeof(th->entries[0]));
+    
+    /* Allocate free slots array */
+    free_slots = new int[new_size];
+    if (th->free_slots) {
+	pj_memcpy(free_slots, th->free_slots, 
+		  FREECNT(th) * sizeof(th->free_slots[0]));
+    }
+    /* Initialize the remaining new area */
+    for (i=FREECNT(th), j=th->max_size; j<new_size; ++i, ++j) {
+	free_slots[i] = j;
+    }
+    for ( ; i<new_size; ++i) {
+	free_slots[i] = -1;
+    }
+    
+    /* Apply */
+    delete [] th->entries;
+    th->entries = entries;
+    th->max_size = new_size;
+    delete [] th->free_slots;
+    th->free_slots = free_slots;
+
+    return PJ_SUCCESS;
+}
+
+/* Allocate and register an entry to timer heap for newly scheduled entry */
+static pj_status_t add_entry(pj_timer_heap_t *th, CPjTimerEntry *entry)
+{
+    pj_status_t status;
+    int slot;
+    
+    /* Check that there's still capacity left in the timer heap */
+    if (FREECNT(th) < 1) {
+	// Grow the timer heap twice the capacity
+	status = realloc_timer_heap(th, th->max_size * 2);
+	if (status != PJ_SUCCESS)
+	    return status;
+    }
+    
+    /* Allocate one free slot. Use LIFO */
+    slot = th->free_slots[FREECNT(th)-1];
+    PJ_ASSERT_RETURN((slot >= 0) && (slot < (int)th->max_size) && 
+		     (th->entries[slot]==NULL), PJ_EBUG);
+    
+    th->free_slots[FREECNT(th)-1] = -1;
+    th->entries[slot] = entry;
+    entry->entry_->_timer_id = slot;
+    ++th->cur_size;
+    
+    return PJ_SUCCESS;
+}
+
+/* Free a slot when an entry's timer has elapsed or cancel */
+static pj_status_t remove_entry(pj_timer_heap_t *th, CPjTimerEntry *entry)
+{
+    int slot = entry->entry_->_timer_id;
+    
+    PJ_ASSERT_RETURN(slot >= 0 && slot < (int)th->max_size, PJ_EBUG);
+    PJ_ASSERT_RETURN(FREECNT(th) < th->max_size, PJ_EBUG);
+    PJ_ASSERT_RETURN(th->entries[slot]==entry, PJ_EBUG);
+    PJ_ASSERT_RETURN(th->free_slots[FREECNT(th)]==-1, PJ_EBUG);
+    
+    th->entries[slot] = NULL;
+    th->free_slots[FREECNT(th)] = slot;
+    entry->entry_->_timer_id = -1;
+    --th->cur_size;
+    
+    return PJ_SUCCESS;
+}
+
 
 CPjTimerEntry::CPjTimerEntry(pj_timer_heap_t *timer_heap,
 			     pj_timer_entry *entry)
-: CActive(PJ_SYMBIAN_TIMER_PRIORITY), timer_heap_(timer_heap), entry_(entry),
+: CActive(PJ_SYMBIAN_TIMER_PRIORITY), entry_(entry), timer_heap_(timer_heap), 
   interval_left_(0)
 {
 }
@@ -132,8 +247,7 @@ void CPjTimerEntry::RunL()
 	return;
     }
     
-    --timer_heap_->cur_size;
-    entry_->_timer_id = NULL;
+    remove_entry(timer_heap_, this);
     entry_->cb(timer_heap_, entry_);
     
     // Finger's crossed!
@@ -142,7 +256,14 @@ void CPjTimerEntry::RunL()
 
 void CPjTimerEntry::DoCancel() 
 {
-     rtimer_.Cancel();
+    /* It's possible that _timer_id is -1, see schedule(). In this case,
+     * the entry has not been added to the timer heap, so don't remove
+     * it.
+     */
+    if (entry_ && entry_->_timer_id != -1)
+	remove_entry(timer_heap_, this);
+    
+    rtimer_.Cancel();
 }
 
 
@@ -157,7 +278,7 @@ PJ_DEF(pj_size_t) pj_timer_heap_mem_size(pj_size_t count)
     return /* size of the timer heap itself: */
            sizeof(pj_timer_heap_t) + 
            /* size of each entry: */
-           (count+2) * (sizeof(pj_timer_entry*)+sizeof(pj_timer_id_t)) +
+           (count+2) * (sizeof(void*)+sizeof(int)) +
            /* lock, pool etc: */
            132;
 }
@@ -170,20 +291,21 @@ PJ_DEF(pj_status_t) pj_timer_heap_create( pj_pool_t *pool,
                                           pj_timer_heap_t **p_heap)
 {
     pj_timer_heap_t *ht;
+    pj_status_t status;
 
     PJ_ASSERT_RETURN(pool && p_heap, PJ_EINVAL);
 
     *p_heap = NULL;
 
     /* Allocate timer heap data structure from the pool */
-    ht = PJ_POOL_ALLOC_T(pool, pj_timer_heap_t);
+    ht = PJ_POOL_ZALLOC_T(pool, pj_timer_heap_t);
     if (!ht)
         return PJ_ENOMEM;
 
-    /* Initialize timer heap sizes */
-    ht->max_size = size;
-    ht->cur_size = 0;
-    ht->max_entries_per_poll = DEFAULT_MAX_TIMED_OUT_PER_POLL;
+    /* Allocate slots */
+    status = realloc_timer_heap(ht, size);
+    if (status != PJ_SUCCESS)
+	return status;
 
     *p_heap = ht;
     return PJ_SUCCESS;
@@ -191,7 +313,23 @@ PJ_DEF(pj_status_t) pj_timer_heap_create( pj_pool_t *pool,
 
 PJ_DEF(void) pj_timer_heap_destroy( pj_timer_heap_t *ht )
 {
-    PJ_UNUSED_ARG(ht);
+    /* Cancel and delete pending active objects */
+    if (ht->entries) {
+	unsigned i;
+	for (i=0; i<ht->max_size; ++i) {
+	    if (ht->entries[i]) {
+		ht->entries[i]->Cancel();
+		delete ht->entries[i];
+		ht->entries[i] = NULL;
+	    }
+	}
+    }
+    
+    delete [] ht->entries;
+    delete [] ht->free_slots;
+    
+    ht->entries = NULL;
+    ht->free_slots = NULL;
 }
 
 PJ_DEF(void) pj_timer_heap_set_lock(  pj_timer_heap_t *ht,
@@ -207,9 +345,9 @@ PJ_DEF(void) pj_timer_heap_set_lock(  pj_timer_heap_t *ht,
 PJ_DEF(unsigned) pj_timer_heap_set_max_timed_out_per_poll(pj_timer_heap_t *ht,
                                                           unsigned count )
 {
-    unsigned old_count = ht->max_entries_per_poll;
-    ht->max_entries_per_poll = count;
-    return old_count;
+    /* Not applicable */
+    PJ_UNUSED_ARG(count);
+    return ht->max_size;
 }
 
 PJ_DEF(pj_timer_entry*) pj_timer_entry_init( pj_timer_entry *entry,
@@ -219,7 +357,7 @@ PJ_DEF(pj_timer_entry*) pj_timer_entry_init( pj_timer_entry *entry,
 {
     pj_assert(entry && cb);
 
-    entry->_timer_id = NULL;
+    entry->_timer_id = -1;
     entry->id = id;
     entry->user_data = user_data;
     entry->cb = cb;
@@ -232,17 +370,24 @@ PJ_DEF(pj_status_t) pj_timer_heap_schedule( pj_timer_heap_t *ht,
 					    const pj_time_val *delay)
 {
     CPjTimerEntry *timerObj;
+    pj_status_t status;
     
     PJ_ASSERT_RETURN(ht && entry && delay, PJ_EINVAL);
     PJ_ASSERT_RETURN(entry->cb != NULL, PJ_EINVAL);
 
     /* Prevent same entry from being scheduled more than once */
-    PJ_ASSERT_RETURN(entry->_timer_id == NULL, PJ_EINVALIDOP);
+    PJ_ASSERT_RETURN(entry->_timer_id < 1, PJ_EINVALIDOP);
 
+    entry->_timer_id = -1;
+    
     timerObj = CPjTimerEntry::NewL(ht, entry, delay);
-    entry->_timer_id = (void*) timerObj;
+    status = add_entry(ht, timerObj);
+    if (status != PJ_SUCCESS) {
+	timerObj->Cancel();
+	delete timerObj;
+	return status;
+    }
     
-    ++ht->cur_size;
     return PJ_SUCCESS;
 }
 
@@ -251,13 +396,15 @@ PJ_DEF(int) pj_timer_heap_cancel( pj_timer_heap_t *ht,
 {
     PJ_ASSERT_RETURN(ht && entry, PJ_EINVAL);
     
-    if (entry->_timer_id != NULL) {
-    	CPjTimerEntry *timerObj = (CPjTimerEntry*) entry->_timer_id;
-    	timerObj->Cancel();
-    	delete timerObj;
-    	entry->_timer_id = NULL;
-    	--ht->cur_size;
-    	return 1;
+    if (entry->_timer_id >= 0 && entry->_timer_id < (int)ht->max_size) {
+    	CPjTimerEntry *timerObj = ht->entries[entry->_timer_id];
+    	if (timerObj) {
+    	    timerObj->Cancel();
+    	    delete timerObj;
+    	    return 1;
+    	} else {
+    	    return 0;
+    	}
     } else {
     	return 0;
     }