Merge r1666468, r1666618, r1674697 from trunk:

mpm_event: Allow for timer events duplicates.
Meanwhile ap[r]_skiplist_add()...

mpm_event: follow up to r1666468.
We only need one compare function for add semantic with apr_skiplist_insert()
and unique timers (pointers). It also should work with apr_skiplist_remove()
and apr_skiplist_find(), be they used some day.

mpm_event: follow up to r1666468 and r1666618.
We don't need to return 0 in the compare function, but for debugging purpose
which we could implement later if necessary (in a separate function).
For now, keep the function simple as in 2.4.x to ease backport, and add a
comment about why we never return 0 here.

Submitted by: ylavic
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1674921 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index d78ba679d69941493db7c12eab42aa26ad728ccc..b39ef66a6c4ab69a68e71c2e60a958bcd2c08037 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,8 @@ Changes with Apache 2.4.13
      calls r:wsupgrade() can cause a child process crash. 
      [Edward Lu <Chaosed0 gmail.com>]
 
+  *) mpm_event: Allow for timer events duplicates. [Jim Jagielski, Yann Ylavic]
+
   *) mod_proxy, mod_ssl, mod_cache_socache, mod_socache_*: Support machine
      readable server-status produced when using the "?auto" query string.
      [Rainer Jung]

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 6a550a1dedd30b851e1a3612a8e08122a8a6da3e..825686cb8031dfd7957b6cdbfb6abd6d42f36542 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -1333,21 +1333,25 @@ static APR_RING_HEAD(timer_free_ring_t, timer_event_t) timer_free_ring;
 
 static apr_skiplist *timer_skiplist;
 
-static int indexing_comp(void *a, void *b)
+/* The following compare function is used by apr_skiplist_insert() to keep the
+ * elements (timers) sorted and provide O(log n) complexity (this is also true
+ * for apr_skiplist_{find,remove}(), but those are not used in MPM event where
+ * inserted timers are not searched nor removed, but with apr_skiplist_pop()
+ * which does use any compare function).  It is meant to return 0 when a == b,
+ * <0 when a < b, and >0 when a > b.  However apr_skiplist_insert() will not
+ * add duplicates (i.e. a == b), and apr_skiplist_add() is only available in
+ * APR 1.6, yet multiple timers could possibly be created in the same micro-
+ * second (duplicates with regard to apr_time_t); therefore we implement the
+ * compare function to return +1 instead of 0 when compared timers are equal,
+ * thus duplicates are still added after each other (in order of insertion).
+ */
+static int timer_comp(void *a, void *b)
 {
-    apr_time_t t1 = (apr_time_t) (((timer_event_t *) a)->when);
-    apr_time_t t2 = (apr_time_t) (((timer_event_t *) b)->when);
+    apr_time_t t1 = (apr_time_t) ((timer_event_t *)a)->when;
+    apr_time_t t2 = (apr_time_t) ((timer_event_t *)b)->when;
     AP_DEBUG_ASSERT(t1);
     AP_DEBUG_ASSERT(t2);
-    return ((t1 < t2) ? -1 : ((t1 > t2) ? 1 : 0));
-}
-
-static int indexing_compk(void *ac, void *b)
-{
-    apr_time_t *t1 = (apr_time_t *) ac;
-    apr_time_t t2 = (apr_time_t) (((timer_event_t *) b)->when);
-    AP_DEBUG_ASSERT(t2);
-    return ((*t1 < t2) ? -1 : ((*t1 > t2) ? 1 : 0));
+    return ((t1 < t2) ? -1 : 1);
 }
 
 static apr_thread_mutex_t *g_timer_skiplist_mtx;
@@ -1374,8 +1378,8 @@ static apr_status_t event_register_timed_callback(apr_time_t t,
     /* XXXXX: optimize */
     te->when = t + apr_time_now();
 
-    /* Okay, insert sorted by when.. */
-    apr_skiplist_insert(timer_skiplist, (void *)te);
+    /* Okay, add sorted by when.. */
+    apr_skiplist_insert(timer_skiplist, te);
 
     apr_thread_mutex_unlock(g_timer_skiplist_mtx);
 
@@ -2237,7 +2241,7 @@ static void child_main(int child_num_arg)
     apr_thread_mutex_create(&g_timer_skiplist_mtx, APR_THREAD_MUTEX_DEFAULT, pchild);
     APR_RING_INIT(&timer_free_ring, timer_event_t, link);
     apr_skiplist_init(&timer_skiplist, pchild);
-    apr_skiplist_set_compare(timer_skiplist, indexing_comp, indexing_compk);
+    apr_skiplist_set_compare(timer_skiplist, timer_comp, timer_comp);
     ap_run_child_init(pchild, ap_server_conf);
 
     /* done with init critical section */