BlackBerry: improve BPS event lifetime management

In QEventDispatcherBlackberry::select(), if an event handler called
through filterEvent() starts a nested event loop by creating a new
QEventLoop, we will recursively enter the select() method again.
However, each time bps_get_event() is called, it destroys the last
event it handed out before returning the next event.  We don't want it
to destroy the event that triggered the nested event loop, since there
may still be more handlers that need to get that event, once the
nested event loop is done and control returns to the outer event loop.

So we move an event to a holding channel, which takes ownership of the
event.  Putting the event on our own channel allows us to manage when
it is destroyed, keeping it alive until we know we are done with it.
Each recursive call of this function needs to have it's own holding
channel, since a channel is a queue, not a stack.

However, a recursive call into the select() method happens very rarely
compared to the many times this method is called.  We don't want to
create a holding channel for each time this method is called, only
when it is called recursively.  Thus we have the instance variable
d->holding_channel to use in the common case.  We keep track of
recursive calls with d->loop_level.  If we are in a recursive call,
then we create a new holding channel for this run.

Change-Id: Ib3584676d2db5a9a3754a1535d5fb6c9e14f5dbb
Reviewed-by: Thomas McGuire <thomas.mcguire@kdab.com>

1 files changed, 87 insertions, 3 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_blackberry.cpp b/src/corelib/kernel/qeventdispatcher_blackberry.cpp
index d9e38b68b2..b9137ec139 100644
--- a/src/corelib/kernel/qeventdispatcher_blackberry.cpp
+++ b/src/corelib/kernel/qeventdispatcher_blackberry.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2012 Research In Motion
+** Copyright (C) 2012 - 2013 BlackBerry Limited. All rights reserved.
 ** Contact: http://www.qt-project.org/legal
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -78,6 +78,19 @@ private:
     int outerChannel;
 };
 
+class BBScopedLoopLevelCounter
+{
+    QEventDispatcherBlackberryPrivate *d;
+
+public:
+    inline BBScopedLoopLevelCounter(QEventDispatcherBlackberryPrivate *p)
+        : d(p)
+    { ++d->loop_level; }
+
+    inline ~BBScopedLoopLevelCounter()
+    { --d->loop_level; }
+};
+
 struct bpsIOHandlerData
 {
     bpsIOHandlerData()
@@ -147,7 +160,8 @@ static int bpsIOHandler(int fd, int io_events, void *data)
 }
 
 QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberryPrivate()
-    : ioData(new bpsIOHandlerData)
+    : loop_level(0)
+    , ioData(new bpsIOHandlerData)
 {
     // prepare to use BPS
     int result = bps_initialize();
@@ -156,6 +170,11 @@ QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberryPrivate()
 
     bps_channel = bps_channel_get_active();
 
+    if (bps_channel_create(&holding_channel, 0) != BPS_SUCCESS) {
+        qWarning("QEventDispatcherBlackberry: bps_channel_create failed");
+        holding_channel = -1;
+    }
+
     // get domain for IO ready and wake up events - ignoring race condition here for now
     if (bpsUnblockDomain == -1) {
         bpsUnblockDomain = bps_register_domain();
@@ -166,6 +185,11 @@ QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberryPrivate()
 
 QEventDispatcherBlackberryPrivate::~QEventDispatcherBlackberryPrivate()
 {
+    if ((holding_channel != -1) &&
+        (bps_channel_destroy(holding_channel) != BPS_SUCCESS)) {
+        qWarning("QEventDispatcherBlackberry: bps_channel_destroy failed");
+    }
+
     // we're done using BPS
     bps_shutdown();
 }
@@ -280,11 +304,21 @@ static inline int timespecToMillisecs(const timespec &tv)
     return (tv.tv_sec * 1000) + (tv.tv_nsec / 1000000);
 }
 
+static inline void destroyHeldBpsEvent(int holding_channel)
+{
+    // Switch to the holding channel and use bps_get_event() to trigger its destruction.  We
+    // don't care about the return value from this call to bps_get_event().
+    BpsChannelScopeSwitcher holdingChannelSwitcher(holding_channel);
+    bps_event_t *held_event = 0;
+    (void)bps_get_event(&held_event, 0);
+ }
+
 int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
                                        timespec *timeout)
 {
     Q_UNUSED(nfds);
     Q_D(QEventDispatcherBlackberry);
+    const BBScopedLoopLevelCounter bbLoopCounter(d);
 
     BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
 
@@ -307,6 +341,31 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
     bps_event_t *event = 0;
     unsigned int eventCount = 0;
 
+    // If an event handler called through filterEvent() starts a nested event loop by creating a
+    // new QEventLoop, we will recursively enter this function again.  However, each time
+    // bps_get_event() is called, it destroys the last event it handed out before returning the
+    // next event.  We don't want it to destroy the event that triggered the nested event loop,
+    // since there may still be more handlers that need to get that event, once the nested event
+    // loop is done and control returns to the outer event loop.
+    //
+    // So we move an event to a holding channel, which takes ownership of the event.  Putting
+    // the event on our own channel allows us to manage when it is destroyed, keeping it alive
+    // until we know we are done with it.  Each recursive call of this function needs to have
+    // it's own holding channel, since a channel is a queue, not a stack.
+    //
+    // However, a recursive call into this function happens very rarely compared to the many
+    // times this function is called.  We don't want to create a holding channel for each time
+    // this function is called, only when it is called recursively.  Thus we have the instance
+    // variable d->holding_channel to use in the common case.  We keep track of recursive calls
+    // with d->loop_level.  If we are in a recursive call, then we create a new holding channel
+    // for this run.
+    int holding_channel = d->holding_channel;
+    if ((d->loop_level > 1) &&
+        Q_UNLIKELY(bps_channel_create(&holding_channel, 0) != BPS_SUCCESS)) {
+        qWarning("QEventDispatcherBlackberry: bps_channel_create failed");
+        holding_channel = -1;
+    }
+
     // Convert timeout to milliseconds
     int timeoutTotal = -1;
     if (timeout)
@@ -331,6 +390,11 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
                 emit awake();
                 filterNativeEvent(QByteArrayLiteral("bps_event_t"), static_cast<void*>(event), 0);
                 emit aboutToBlock();
+
+                if (Q_LIKELY(holding_channel != -1)) {
+                    // We are now done with this BPS event.  Destroy it.
+                    destroyHeldBpsEvent(holding_channel);
+                }
             }
 
             // Update the timeout
@@ -370,6 +434,12 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
         if (bps_event_get_domain(event) == bpsUnblockDomain) {
             timeoutTotal = 0;   // in order to immediately drain the event queue of native events
             event = 0;          // (especially touch move events) we don't break out here
+        } else {
+            // Move the event to our holding channel so we can manage when it is destroyed.
+            if (Q_LIKELY(holding_channel != 1) &&
+                Q_UNLIKELY(bps_channel_push_event(holding_channel, event) != BPS_SUCCESS)) {
+                qWarning("QEventDispatcherBlackberry: bps_channel_push_event failed");
+            }
         }
 
         ++eventCount;
@@ -379,12 +449,26 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
         const unsigned int maximumEventCount = 12;
         if (Q_UNLIKELY((eventCount > maximumEventCount && timeoutLeft == 0)
                        || !QElapsedTimer::isMonotonic())) {
-            if (event)
+            if (event) {
                 filterNativeEvent(QByteArrayLiteral("bps_event_t"), static_cast<void*>(event), 0);
+
+                if (Q_LIKELY(holding_channel != -1)) {
+                    // We are now done with this BPS event.  Destroy it.
+                    destroyHeldBpsEvent(holding_channel);
+                }
+            }
             break;
         }
     }
 
+    // If this was a recursive call into this function, a new holding channel was created for
+    // this run, so destroy it now.
+    if ((holding_channel != d->holding_channel) &&
+        Q_LIKELY(holding_channel != -1) &&
+        Q_UNLIKELY(bps_channel_destroy(holding_channel) != BPS_SUCCESS)) {
+        qWarning("QEventDispatcherBlackberry: bps_channel_destroy failed");
+    }
+
     // the number of bits set in the file sets
     return d->ioData->count;
 }