I expect the attached patch to fix the likely bug reported by Hans.
As previously indicated, we should be able to either rearm the next
timer or disarm all timers in the arm_timerfd_for_next_timeout() call of
io.c, especially as we always issue it with the flying_transfers locked.
Note that this patch may result in the timerfd being disarmed twice in
handle_timerfd_trigger(). Apart from a slight overhead, I don't expect
that to be a problem. Especially, the timerfd_settime() man page does
not indicate that disarming a timerfd twice will result in an error
code. Of course, to avoid that we could try to remove the unconditional
disarm call at the beginning of handle_timerfd_trigger(), but I believe
we very much want to have it before issuing handle_timeouts_locked(), to
avoid potentially accumulating a whole bunch of timerfd events, while we
already are in the process of checking for timeouts anyway.
Also, I haven't tested this change under the conditions where I'd expect
the issue to manifest itself, so there is a possibility the patch (or
the reality of the problem) is still off.
Regards,
/Pete
>From 04ad04771e7cb6a50f6a0a5eb2481ee6989ebe0e Mon Sep 17 00:00:00 2001
From: Pete Batard <p...@akeo.ie>
Date: Tue, 10 Jul 2012 01:59:45 +0100
Subject: [PATCH] Core: Fix unconditional disarming of timerfd
* Existing code appears to disarm the timerfd always, which
would cancel all pending timeouts as soon as one packet completes.
* This fix moves the disarming of the timerfd to
arm_timerfd_for_next_timeout(), where it is now done conditionally.
* Issue reported by Hans de Goede. For more info, see:
https://sourceforge.net/mailarchive/message.php?msg_id=29442693
---
libusb/io.c | 22 +++++++---------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index d06d375..9296ff3 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1403,7 +1403,7 @@ static int arm_timerfd_for_next_timeout(struct
libusb_context *ctx)
/* if we've reached transfers of infinite timeout, then we have
no
* arming to do */
if (!timerisset(cur_tv))
- return 0;
+ return disarm_timerfd(ctx);
/* act on first transfer that is not already cancelled */
if (!(transfer->flags & USBI_TRANSFER_TIMED_OUT)) {
@@ -1418,14 +1418,9 @@ static int arm_timerfd_for_next_timeout(struct
libusb_context *ctx)
}
}
- return 0;
+ return disarm_timerfd(ctx);
}
#else
-static int disarm_timerfd(struct libusb_context *ctx)
-{
- (void)ctx;
- return 0;
-}
static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
{
(void)ctx;
@@ -1457,17 +1452,14 @@ int usbi_handle_transfer_completion(struct
usbi_transfer *itransfer,
usbi_mutex_lock(&ctx->flying_transfers_lock);
list_del(&itransfer->list);
- if (usbi_using_timerfd(ctx))
- r = arm_timerfd_for_next_timeout(ctx);
- usbi_mutex_unlock(&ctx->flying_transfers_lock);
-
if (usbi_using_timerfd(ctx)) {
- if (r < 0)
- return r;
- r = disarm_timerfd(ctx);
- if (r < 0)
+ r = arm_timerfd_for_next_timeout(ctx);
+ if (r < 0) {
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
return r;
+ }
}
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
if (status == LIBUSB_TRANSFER_COMPLETED
&& transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) {
--
1.7.11.msysgit.0
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
