Package: libsigcx
Version: 0.6.5-2
Severity: grave
Tags: patch, upstream
There are multiple, serious event handling bugs in the dispatch code of
libsigcx that are severe enough to make the library unusable for
programs not using the GTK event loop. A patch to fix them was supplied
to the upstream author, who is also the package maintainer, a year ago
who said they would go in as 0.6.7, but then stopped responding to
email. Around 10 emails over the past year have gone unanswered, likely
as the author/maintainer is no longer interested in this package.
As attempts have failed to get these fixes pushed upstream, I would like
to see these bugs fixed in Debian at least, so this bug is to apply the
attached patch. Its header contains more details about what's fixed,
where the fixes come from (it's a merge of an arch branch I've been
maintaining), and the history of this patch re the thread with the
author/maintainer.
I've prepared a NMU against the current 0.6.5-2 in unstable but I'm not
a DD (but want to be :)) so can't upload it. If that's an easier path
than applying the attached patch to make a -3, you can find the source
package (0.6.5-2.1) at:
http://activecampus2.ucsd.edu/apt/sarge/libsigcx-0.6
Note the binary package is compiled against sarge, not unstable. If
needed, I can compile it against unstable for you.
Cumulative patch from patch-1, patch-2, patch-3 from arch,
branch: [EMAIL PROTECTED]/libsigcx--events--0.1
archive: http://wiisard.org/~swbrown/{archives}/2005-fixes
patch-1:
Fix iterator corruption and calling removed handlers when removing event
handlers from event handlers, and fix not select()ing on the first pass on
file event handlers added from event handlers on the same pass.
patch-2:
Fix spurrious fd events due to remove() not removing the fd immediately from
the select sets causing select() to fail which isn't caught, leading to invalid
sets being used for one pass.
patch-3:
Remove unused fevent variable.
Author/maintainer stopped responding to email and dev list email shortly after
I prepared patch-1 for merging - patch-1 is almost a year old and fixes
critical bugs. Maintainer has been emailed around 10 times over the past year
with no response other than the original saying this patch would be merged for
a 0.6.7. See thread "Event handling fixes" at SourceForge (it breaks the
threading, but here's a link to most of the thread):
http://sourceforge.net/mailarchive/forum.php?thread_id=7867763&forum_id=11540
These are critical event handling bugs that need fixed.
- Steven Brown <[EMAIL PROTECTED]>
diff -ruN libsigcx-0.6.5-pristine/sigcx/dispatch.cc
libsigcx-0.6.5/sigcx/dispatch.cc
--- libsigcx-0.6.5-pristine/sigcx/dispatch.cc 2005-04-10 09:39:18.000000000
-0700
+++ libsigcx-0.6.5/sigcx/dispatch.cc 2006-06-05 14:48:11.198328848 -0700
@@ -164,7 +164,7 @@
StandardDispatcher::TimerEvent::TimerEvent(const Handler& _cb,
const TimeVal& tmout)
- : cb(_cb)
+ : cb(_cb), deleted(false)
{
expiration.get_current_time();
@@ -227,16 +227,18 @@
void StandardDispatcher::remove(HandlerID id)
{
Guard guard(mutex_);
-
+
+ // Queue the removal as to not damage the iterators if we're being called by
+ // a callback in run().
+
TMHandlerMap::iterator tm_it = tm_handlers_.find(id);
if (tm_it != tm_handlers_.end())
{
TMEventMap::iterator ev_it = tm_it->second;
- tm_handlers_.erase(tm_it);
- tm_events_.erase(ev_it);
+ ev_it->first.deleted = true;
return;
}
-
+
FDHandlerMap::iterator fd_it = fd_handlers_.find(id);
if (fd_it != fd_handlers_.end())
{
@@ -249,6 +251,25 @@
if (fevent.ev == Except && FD_ISSET(fevent.fd, &ex_fds_))
FD_CLR(fevent.fd, &ex_fds_);
+ fevent.deleted = true;
+ }
+}
+
+void StandardDispatcher::real_remove(HandlerID id)
+{
+
+ TMHandlerMap::iterator tm_it = tm_handlers_.find(id);
+ if (tm_it != tm_handlers_.end())
+ {
+ TMEventMap::iterator ev_it = tm_it->second;
+ tm_handlers_.erase(tm_it);
+ tm_events_.erase(ev_it);
+ return;
+ }
+
+ FDHandlerMap::iterator fd_it = fd_handlers_.find(id);
+ if (fd_it != fd_handlers_.end())
+ {
fd_handlers_.erase(fd_it);
return;
}
@@ -269,10 +290,6 @@
// We set it here, it may be unset in a handler by calling exit
do_exit_ = false;
- fd_set rd = rd_fds_;
- fd_set wr = wr_fds_;
- fd_set ex = ex_fds_;
-
// We service timeouts in two passes: first collect all expired
// timeouts, then invoke them. If we did this in one pass, the
// callbacks could starve the FD handlers by adding new timeouts
@@ -300,7 +317,12 @@
try
{
UnGuard unguard(mutex_);
- const_cast<Handler&>(ev_it->first.cb)();
+ // Only call it if it isn't queued to be removed - a timeout function
+ // could have tried to remove the one we're looking at earlier in this
+ // pass.
+ if (!ev_it->first.deleted) {
+ const_cast<Handler&>(ev_it->first.cb)();
+ }
}
catch (...)
{
@@ -317,6 +339,13 @@
if (do_exit_)
break;
+ // Initialize the fd sets here, as a timeout callback could have added a
+ // new FileEvent. Otherwise, we'd not be able to wake up from select as
+ // the fd wouldn't be in the sets.
+ fd_set rd = rd_fds_;
+ fd_set wr = wr_fds_;
+ fd_set ex = ex_fds_;
+
if (tm_events_.size() == 0)
{
UnGuard unguard (mutex_);
@@ -360,11 +389,25 @@
if (event != All)
{
UnGuard unguard(mutex_);
- fevent.cb();
+ // Only call it if it isn't queued to be removed - a fevent function
+ // could have tried to remove the one we're looking at earlier.
+ if (!fevent.deleted)
+ fevent.cb();
}
if (do_exit_)
break;
}
+
+ // Delete any file events that are marked deleted.
+ for (j = fd_handlers_.begin(); j != fd_handlers_.end(); )
+ {
+ FileEvent& fevent = j->second;
+ HandlerID id = j->first;
+ ++j;
+ if (fevent.deleted)
+ real_remove(id);
+ }
+
} while (infinite && !do_exit_);
return do_exit_;
diff -ruN libsigcx-0.6.5-pristine/sigcx/dispatch.h
libsigcx-0.6.5/sigcx/dispatch.h
--- libsigcx-0.6.5-pristine/sigcx/dispatch.h 2003-04-25 04:06:36.000000000
-0700
+++ libsigcx-0.6.5/sigcx/dispatch.h 2006-06-05 14:48:06.685846436 -0700
@@ -190,24 +190,28 @@
virtual void exit();
virtual void move(Dispatcher& d);
virtual bool idle() const;
+ protected:
+ virtual void real_remove(HandlerID id);
private:
struct FileEvent
{
Handler cb;
Event ev;
int fd;
+ mutable bool deleted;
- FileEvent() {}
+ FileEvent() : deleted(false) {}
FileEvent(const Handler& _cb, Event _ev, int _fd)
- : cb(_cb), ev(_ev), fd(_fd)
+ : cb(_cb), ev(_ev), fd(_fd), deleted(false)
{}
};
struct TimerEvent
{
Handler cb;
TimeVal expiration;
+ mutable bool deleted;
- TimerEvent() {}
+ TimerEvent() : deleted(false) {}
TimerEvent(const Handler& _cb, const TimeVal& tv);
bool operator<(const TimerEvent& t) const {
