Changeset 3034 for pjproject/trunk

Timestamp:

Dec 16, 2009 1:30:34 PM (15 years ago)

Author:

bennylp

Message:

Ticket #1004: Symbian timer heap fix

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

Location:

pjproject/trunk/pjlib

Files:

• include/pj/timer.h (modified) (1 diff)
• src/pj/timer_symbian.cpp (modified) (16 diffs)

pjproject/trunk/pjlib/include/pj/timer.h

@@ -65 +65 @@
  * 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
 
 /** 

pjproject/trunk/pjlib/src/pj/timer_symbian.cpp

@@ -32 +32 @@
 #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;
 
 /**
@@ -44 +54 @@
     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.
@@ -56 +69 @@
 {
 public:
+    pj_timer_entry  *entry_;
+    
     static CPjTimerEntry* NewL( pj_timer_heap_t *timer_heap,
                                 pj_timer_entry *entry,
@@ -67 +82 @@
 private:        
     pj_timer_heap_t *timer_heap_;
-    pj_timer_entry  *entry_;
     RTimer           rtimer_;
     pj_uint32_t      interval_left_;
@@ -76 +90 @@
 };
 
+//////////////////////////////////////////////////////////////////////////////
+/*
+ * 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)
 {
@@ -133 +248 @@
     }
     
-    --timer_heap_->cur_size;
-    entry_->_timer_id = NULL;
+    remove_entry(timer_heap_, this);
     entry_->cb(timer_heap_, entry_);
     
@@ -143 +257 @@
 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();
 }
 
@@ -158 +279 @@
            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;
@@ -171 +292 @@
 {
     pj_timer_heap_t *ht;
+    pj_status_t status;
 
     PJ_ASSERT_RETURN(pool && p_heap, PJ_EINVAL);
@@ -177 +299 @@
 
     /* 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;
@@ -192 +314 @@
 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;
 }
 
@@ -208 +346 @@
                                                           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;
 }
 
@@ -220 +358 @@
     pj_assert(entry && cb);
 
-    entry->_timer_id = NULL;
+    entry->_timer_id = -1;
     entry->id = id;
     entry->user_data = user_data;
@@ -233 +371 @@
 {
     CPjTimerEntry *timerObj;
+    pj_status_t status;
     
     PJ_ASSERT_RETURN(ht && entry && delay, PJ_EINVAL);
@@ -238 +377 @@
 
     /* 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;
-    
-    ++ht->cur_size;
+    status = add_entry(ht, timerObj);
+    if (status != PJ_SUCCESS) {
+        timerObj->Cancel();
+        delete timerObj;
+        return status;
+    }
+    
     return PJ_SUCCESS;
 }
@@ -252 +397 @@
     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;