Opened 5 months ago

Closed 4 months ago

#2176 closed defect (fixed)

Create stress test for timer heap

Reported by: nanang Owned by:
Priority: normal Milestone: release-2.9
Component: pjlib Version: trunk
Keywords: Cc:
Backport to 1.x milestone: Backported: no

Description (last modified by nanang)

There have been some reports about crash in timer heap (there was #2172, but reported that crash still occurs even after integrating the patch). Creating a stress test for timer heap to reproduce the issue could be useful in identifying the problem and verifying a future solution.

Few notes after running the stress test

  • The stress test could reproduce the previously suspected issue of "pop_freelist()+push_freelist() after remove_node() in pj_timer_heap_poll()", here is a bit more description on it:

pop_freelist() will only preserve the timer_id, not the timer entry as the timer entry node has been completely detached from the heap. it may cause crash, as the number of free timer_id is less than timer heap slot, e.g: if the app has 4 timer polling threads, there can be 4 less free timer_id than timer heap slot, while the timer heap slot will grow only when there are 2 free timer heap slot left.

So r5934 removes pop_freelist() + push_freelist() after remove_node().

  • The issue described in #2172 (double destroy after cancel_timer() return code not checked) could not be reproduced, but later after a further investigation, found out that it may actually be a false alarm because the poll() sets timer entry group lock to NULL (within the same timer lock block as remove_node()) before invoking callback, so it should be safe from the double destroy issue.

Change History (3)

comment:1 Changed 4 months ago by nanang

In 5933:

Re #2176: added timer stress test into pjlib-test.

comment:2 Changed 4 months ago by nanang

In 5934:

Re #2176: Removed pop_freelist() + push_freelist() after remove_node() as they are not only unnecessary, they cause problem.

comment:3 Changed 4 months ago by nanang

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.