BlackBerry: Revised error handling in event dispatcher

Added an upper bound check for socket notifier file descriptors. Too
high FDs have been a real source of failure.
Added compiler hints to allow error-free code path run faster (errors
are highly unlikely) and adjusted warning messages (some were misleading
and too long).

Change-Id: I1c9c41f5d006ca9d3a28214c3a464555b8a1c71f
Reviewed-by: Thomas McGuire <thomas.mcguire@kdab.com>
This commit is contained in:
Bernd Weimer 2013-05-08 14:45:03 +02:00 committed by The Qt Project
parent 3be197881f
commit ebd5de126c

View File

@ -126,16 +126,16 @@ static int bpsIOHandler(int fd, int io_events, void *data)
// create unblock event // create unblock event
bps_event_t *event; bps_event_t *event;
int result = bps_event_create(&event, bpsUnblockDomain, 0, NULL, NULL); int result = bps_event_create(&event, bpsUnblockDomain, 0, NULL, NULL);
if (result != BPS_SUCCESS) { if (Q_UNLIKELY(result != BPS_SUCCESS)) {
qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_event_create() failed"); qWarning("QEventDispatcherBlackberry: bps_event_create failed");
return BPS_FAILURE; return BPS_FAILURE;
} }
// post unblock event to our thread; in this callback the bps channel is // post unblock event to our thread; in this callback the bps channel is
// guaranteed to be the same that was active when bps_add_fd was called // guaranteed to be the same that was active when bps_add_fd was called
result = bps_push_event(event); result = bps_push_event(event);
if (result != BPS_SUCCESS) { if (Q_UNLIKELY(result != BPS_SUCCESS)) {
qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_push_event() failed"); qWarning("QEventDispatcherBlackberry: bps_push_event failed");
bps_event_destroy(event); bps_event_destroy(event);
return BPS_FAILURE; return BPS_FAILURE;
} }
@ -149,16 +149,16 @@ QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberryPrivate()
{ {
// prepare to use BPS // prepare to use BPS
int result = bps_initialize(); int result = bps_initialize();
if (result != BPS_SUCCESS) if (Q_UNLIKELY(result != BPS_SUCCESS))
qFatal("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_initialize() failed"); qFatal("QEventDispatcherBlackberry: bps_initialize failed");
bps_channel = bps_channel_get_active(); bps_channel = bps_channel_get_active();
// get domain for IO ready and wake up events - ignoring race condition here for now // get domain for IO ready and wake up events - ignoring race condition here for now
if (bpsUnblockDomain == -1) { if (bpsUnblockDomain == -1) {
bpsUnblockDomain = bps_register_domain(); bpsUnblockDomain = bps_register_domain();
if (bpsUnblockDomain == -1) if (Q_UNLIKELY(bpsUnblockDomain == -1))
qWarning("QEventDispatcherBlackberryPrivate::QEventDispatcherBlackberry: bps_register_domain() failed"); qWarning("QEventDispatcherBlackberry: bps_register_domain failed");
} }
} }
@ -200,21 +200,26 @@ void QEventDispatcherBlackberry::registerSocketNotifier(QSocketNotifier *notifie
Q_ASSERT(notifier); Q_ASSERT(notifier);
Q_D(QEventDispatcherBlackberry); Q_D(QEventDispatcherBlackberry);
BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
// Register the fd with bps
int sockfd = notifier->socket(); int sockfd = notifier->socket();
int type = notifier->type(); int type = notifier->type();
qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd; qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
int io_events = ioEvents(sockfd); if (Q_UNLIKELY(sockfd >= FD_SETSIZE)) {
qWarning() << "QEventDispatcherBlackberry: cannot register QSocketNotifier (fd too high)"
if (io_events) << sockfd;
bps_remove_fd(sockfd); return;
}
// Call the base Unix implementation. Needed to allow select() to be called correctly // Call the base Unix implementation. Needed to allow select() to be called correctly
QEventDispatcherUNIX::registerSocketNotifier(notifier); QEventDispatcherUNIX::registerSocketNotifier(notifier);
// Register the fd with bps
BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
int io_events = ioEvents(sockfd);
if (io_events)
bps_remove_fd(sockfd);
switch (type) { switch (type) {
case QSocketNotifier::Read: case QSocketNotifier::Read:
qEventDispatcherDebug << "Registering" << sockfd << "for Reads"; qEventDispatcherDebug << "Registering" << sockfd << "for Reads";
@ -231,44 +236,41 @@ void QEventDispatcherBlackberry::registerSocketNotifier(QSocketNotifier *notifie
break; break;
} }
errno = 0; const int result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data());
int result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data()); if (Q_UNLIKELY(result != BPS_SUCCESS))
qWarning() << "QEventDispatcherBlackberry: bps_add_fd failed";
if (result != BPS_SUCCESS)
qWarning() << Q_FUNC_INFO << "bps_add_fd() failed" << strerror(errno) << "code:" << errno;
} }
void QEventDispatcherBlackberry::unregisterSocketNotifier(QSocketNotifier *notifier) void QEventDispatcherBlackberry::unregisterSocketNotifier(QSocketNotifier *notifier)
{ {
Q_D(QEventDispatcherBlackberry); Q_D(QEventDispatcherBlackberry);
BpsChannelScopeSwitcher channelSwitcher(d->bps_channel); int sockfd = notifier->socket();
qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
if (Q_UNLIKELY(sockfd >= FD_SETSIZE)) {
qWarning() << "QEventDispatcherBlackberry: cannot unregister QSocketNotifier" << sockfd;
return;
}
// Allow the base Unix implementation to unregister the fd too // Allow the base Unix implementation to unregister the fd too
QEventDispatcherUNIX::unregisterSocketNotifier(notifier); QEventDispatcherUNIX::unregisterSocketNotifier(notifier);
// Unregister the fd with bps // Unregister the fd with bps
int sockfd = notifier->socket(); BpsChannelScopeSwitcher channelSwitcher(d->bps_channel);
qEventDispatcherDebug << Q_FUNC_INFO << "fd =" << sockfd;
const int io_events = ioEvents(sockfd); const int io_events = ioEvents(sockfd);
int result = bps_remove_fd(sockfd); int result = bps_remove_fd(sockfd);
if (result != BPS_SUCCESS) if (Q_UNLIKELY(result != BPS_SUCCESS))
qWarning() << Q_FUNC_INFO << "bps_remove_fd() failed" << sockfd; qWarning() << "QEventDispatcherBlackberry: bps_remove_fd failed" << sockfd;
// if no other socket notifier is watching sockfd, our job ends here
/* if no other socket notifier is
* watching sockfd, our job ends here
*/
if (!io_events) if (!io_events)
return; return;
errno = 0;
result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data()); result = bps_add_fd(sockfd, io_events, &bpsIOHandler, d->ioData.data());
if (result != BPS_SUCCESS) { if (Q_UNLIKELY(result != BPS_SUCCESS))
qWarning() << Q_FUNC_INFO << "bps_add_fd() failed" << strerror(errno) << "code:" << errno; qWarning("QEventDispatcherBlackberry: bps_add_fd error");
}
} }
static inline int timespecToMillisecs(const timespec &tv) static inline int timespecToMillisecs(const timespec &tv)
@ -352,8 +354,8 @@ int QEventDispatcherBlackberry::select(int nfds, fd_set *readfds, fd_set *writef
// Wait for event or file to be ready // Wait for event or file to be ready
event = 0; event = 0;
const int result = bps_get_event(&event, timeoutLeft); const int result = bps_get_event(&event, timeoutLeft);
if (result != BPS_SUCCESS) if (Q_UNLIKELY(result != BPS_SUCCESS))
qWarning("QEventDispatcherBlackberry::select: bps_get_event() failed"); qWarning("QEventDispatcherBlackberry bps_get_event failed");
if (!event) // In case of !event, we break out of the loop to let Qt process the timers if (!event) // In case of !event, we break out of the loop to let Qt process the timers
break; // (since timeout has expired) and socket notifiers that are now ready. break; // (since timeout has expired) and socket notifiers that are now ready.
@ -385,13 +387,13 @@ void QEventDispatcherBlackberry::wakeUp()
Q_D(QEventDispatcherBlackberry); Q_D(QEventDispatcherBlackberry);
if (d->wakeUps.testAndSetAcquire(0, 1)) { if (d->wakeUps.testAndSetAcquire(0, 1)) {
bps_event_t *event; bps_event_t *event;
if (bps_event_create(&event, bpsUnblockDomain, 0, 0, 0) == BPS_SUCCESS) { if (Q_LIKELY(bps_event_create(&event, bpsUnblockDomain, 0, 0, 0) == BPS_SUCCESS)) {
if (bps_channel_push_event(d->bps_channel, event) == BPS_SUCCESS) if (Q_LIKELY(bps_channel_push_event(d->bps_channel, event) == BPS_SUCCESS))
return; return;
else else
bps_event_destroy(event); bps_event_destroy(event);
} }
qWarning("QEventDispatcherBlackberryPrivate::wakeUp failed"); qWarning("QEventDispatcherBlackberry: wakeUp failed");
} }
} }