request for release with r524 (and r525, r526)

[Eric Wong]

Hello, r524 in trunk fixes a bug I encountered in my application.

I use a large ident value (INT_MAX+1) so an event can't be confused
with a file descriptor, but the truncation to short caused
knote_lookup() to fail on me.

I was mistaken and thought 1.0.5 included this fix[1], but it did not...

[1] - http://bugs.debian.org/669572

[Mark Heily]

On 04/19/2012 10:52 PM, Eric Wong wrote:
> Hello, r524 in trunk fixes a bug I encountered in my application.
>
> I use a large ident value (INT_MAX+1) so an event can't be confused
> with a file descriptor, but the truncation to short caused
> knote_lookup() to fail on me.

Thanks for the bug report. This is fixed in version 1.0.6.

- Mark

[Eric Wong]

Mark Heily <ma...@heily.com> wrote:
> Thanks for the bug report. This is fixed in version 1.0.6.

Thanks for the quick turnaround, solved my problem nicely :)

Btw, is kevent() (with a longish timeout) supposed to be a cancellation
point? I may just rework my app to not rely on pthread_cancel(),
though.

[Mark Heily]

On 04/20/2012 05:02 AM, Eric Wong wrote:
> Btw, is kevent() (with a longish timeout) supposed to be a cancellation
> point? I may just rework my app to not rely on pthread_cancel(),
> though.

libkqueue does not currently support thread cancellation. If you
asynchronously cancel a thread while is is executing kevent(), bad things
(such as deadlock) would occasionally happen.

According to the NetBSD pthread_testcancel() manpage, kevent() is supposed to
be a cancellation point. OTOH, this is not mentioned in the corresponding
FreeBSD manpage. It seems reasonable to me to make kevent() a cancellation
point, so I think we should add this to libkqueue.

I have added a couple entries to the BUGS file to document the general idea:

* kqueue() should defer thread cancellation until the end.

* kevent() should defer thread cancellation and call pthread_testcancel()
before and after the call to kevent_wait(). This may require changing the
way that EINTR is handled, to make sure that the EINTR is propagated up
the call stack to kevent().

It's not a trivial fix, so don't expect it any time soon. It also depends on
the underlying syscalls like epoll() and port_get() being cancellation points,
which may or may not be true. I don't think this will be 100% supported and
bug-free on all platforms, given some limited initial research [1][2].

The best option is to avoid using thread cancellation, IMHO. There are plenty
of alternatives for one thread to interrupt a long-running kevent() call in a
different thread, such as using a socketpair for IPC.

Regards,

- Mark

[1] http://mail.opensolaris.org/pipermail/on-discuss/2008-December/000547.html

[2]
http://read.cs.ucla.edu/gitweb?p=click;a=commitdiff;h=0c38c21413ff71540ab98aadcf94c18885659281

[Eric Wong]

Mark Heily <ma...@heily.com> wrote:
> It also depends on the underlying syscalls like epoll() and port_get()
> being cancellation points,

epoll_wait() is a cancellation point (since glibc 2.4).

> The best option is to avoid using thread cancellation, IMHO. There are plenty
> of alternatives for one thread to interrupt a long-running kevent() call in a
> different thread, such as using a socketpair for IPC.

Agreed!

[Mark Heily]

On 04/20/2012 05:02 AM, Eric Wong wrote:

> Btw, is kevent() (with a longish timeout) supposed to be a cancellation
> point? I may just rework my app to not rely on pthread_cancel(),
> though.

I just committed basic support for thread cancellation in r544 on the stable
branch. This means that cancelling a thread while it is executing kevent() or
kqueue() will not corrupt the internal state of libkqueue.

Unfortunately, there are several places in the code that automatically retry
after receiving EINTR. Here's an example:

src/linux/timer.c:

135 for (;;) {
136 nret = epoll_wait(filt->kf_pfd, &epevt[0], nevents, 0);
137 if (nret < 0) {
138 if (errno == EINTR)
139 continue;
140 dbg_perror("epoll_wait");
141 return (-1);
142 } else {
143 break;
144 }
145 }

All of these will need to be changed to propogate the EINTR return code all
the way back up to kevent(). Until this is fixed, it is possible that the
thread will go back to sleep instead of processing the cancellation request.

This style of EINTR handling is typical for applications, but should be
considered a bug in libkqueue, as all EINTR return codes should be returned to
the caller.

- Mark

[Eric Wong]

Mark Heily <ma...@heily.com> wrote:
> On 04/20/2012 05:02 AM, Eric Wong wrote:
> > Btw, is kevent() (with a longish timeout) supposed to be a cancellation
> > point? I may just rework my app to not rely on pthread_cancel(),
> > though.
>
> I just committed basic support for thread cancellation in r544 on the stable
> branch. This means that cancelling a thread while it is executing kevent() or
> kqueue() will not corrupt the internal state of libkqueue.

Thanks, looks good to me even though I've worked around it in my own app
in a similar fashion (and using NOTE_TRIGGER to kick threads out of
kevent() sleep).