Merely getting an internal notification event from having an event
added or deleted from another thread should not cause
event_base_loop(base, EVLOOP_ONCE) to exit; previously, it did.
When using the signal.c signal backend, Libevent currently only allows
one event_base to actually receive signals at a time. (This has been
the behavior since at least 1.4 and probably much earlier.) Now, we
detect and warn if you're likely to be racing about which signal goes
to which thread.
We also add a lock to control modifications of the evsig_base field,
to avoid race conditions like those found by Jason Toffaletti.
Also, more comments. Comments are good.
The trick here is that if we already told the base to wake up, and it
hasn't woken up yet, we don't need to tell it to wake up again. This
should help lots with inherently multithreaded code like IOCP.
If threads queue callbacks while event_process_deferred_callbacks is
running, the loop may spin long enough to significantly skew timers.
A unit test stressing this behavior is also in this commit.
- Increment reference count of bufferevents before initiating overlapped
operations to prevent the destructor from being called while operations
are pending. The only portable way of canceling overlapped ops is to
close the socket.
- Translate error codes to WSA* codes.
- Better handling of errors.
- Add an interface to add and del "virtual" events. Because IOCP
bufferevents don't register any events with the base, the event loop
has no way of knowing they exist. This causes the loop to terminate
prematurely. event_base_{add,del}_virtual increment/decrement base's
event count so the loop runs while there are any enabled IOCP
bufferevents.
After a fork, you want subthreads to wake up the event_base in the
child process, not to have the child process and the main process
fight over who wakes up whom.
Related to a problem found by Nicholas Marriott while debugging
3048812.
Nicholas Marriott identifies an issue where a signal socketpair
doesn't get recreated if the event backend doesn't set event_reinit.
See bug 3048812
There may be a similar issue with respect to th_notify_fd
Avi Bab correctly noted as bug 3044479 the fact that any thread
blocking on current_event_lock will do so while holding
th_base_lock, making it impossible for the currently running event's
callback to call any other functions that require th_base_lock.
This patch switches the current_event_lock code to instead use a
condition variable that we wait on if we're trying to mess with
a currently-executing event, and that we signal when we're done
executing a callback if anybody is waiting on it.
To be fair, when char can be signed, if toupper doesn't take negative
characters, toupper(char) is a very bad idea. So let's just use the
nice safe EVUTIL_TOUPPER instead. (It explicitly only upcases ASCII,
but we only use it for identifiers that we know to be ASCII anyway).
There was previously no lock protecting the signal event's
ev_ncalls/ev_pncalls fields, which were accessed by all of
event_signal_closure, event_add_internal, event_del_internal, and
event_active_nolock. This patch fixes this race by using the
current_event_lock in the same way it's used to prevent
event_del_internal from touching an event that's currently running.
The problem was that the thread doing the notification could block on
write in evthread_notify_base_default while holding the th_base_lock.
The main thread would never drain th_notify_fd[0], since it would need
th_base_lock to actually trigger events.
The "current_base" symbol was never actually declared in an exported
header; it's hideously deprecated, and it was the one remaining
exported symbol (fwict) that was prefixed with neither ev nor
bufferevent nor _ev nor _bufferevent.
codesearch.google.com turns up no actual attempts to use our
current_base from outside libevent.
Our mm_malloc, mm_calloc, etc functions were all exported, since C
hasn't got a nice portable way to say "we want to use this function
inside our library but not export it to others". But they apparently
conflict with anything else that calls its symbols mm_*, as libmm does.
This patch renames the mm_*() functions to event_mm_*_(, and defines
maros in mm_internal so that all the code we have that uses mm_*()
will still work. New code should also prefer the mm_*() macro names.
Reported by Gernot Tenchio. Fixes sf bug 2996541
Every current BSD system providing TAILQ_* macros define
TAILQ_FOREACH_REVERSE in this order:
TAILQ_FOREACH_REVERSE(var, head, field, headname)
However, libevent defines it in another order:
TAILQ_FOREACH_REVERSE(var, head, headname, field)
Here's a trivial patch to have libevent compatible with stock queue.h headers.
-Frank.
[From sourceforge patch 2995179. codesearch.google.com confirms that
the only people defining TAILQ_FOREACH_REVERSE our way are people
using it in a compatibility header like us. Did we copy this from
OpenSSH or something?]
-Nick
Debug mode needs to be enabled before any event is setup or any
event_base is created. Otherwise, we will not have recorded when events
were first setup or added, and so it will look like a bug later when we
delete or free them.
I have already confused myself because of this requirement, so let's
make Libevent catch it for the next poor forgetful developer like me.
Calling event_base_loop on a base from inside a callback invoked by
that same base, or from two threads at once, has long been a way to
get exceedingly hard-to-diagnose errors. This patch adds code to
detect such reentrant invocatinos, and exit quickly with a warning
that should explain what went wrong.
On Linux, we use only one fd to do main-thread signaling (since we have
eventfd()), so we don't need to close th_notify_fd[1] as we would if we were
using a socketpair.
Remeber, win32 has a socket type that's actually a handle, so if
there's a chance that code is run on win32, we can't use "int" as the
socket type.
This isn't a blind search-and-replace: sometimes an fd is really in
fact for a file, and not a socket at all.
When we fixed persistent timeouts to make them reset themselves
based on the previous scheduled time rather than the current
time... we made them do so regardless of whether the event was
triggering because of a timeout or not!
This was of course bogus. When a _timeout_ triggers, we should
schedule the event for N seconds based on the last
_schedule_ time... but when IO triggers, we should reset the
timeout for N seconds after now.
I found these by adding an EVENT_BASE_ASSERT_LOCKED() call to most
of the functions in event.c that can only be called while holding
the lock.
event_reinit() never grabbed the lock, but it needed to.
event_persist_closure accessed the base to call event_add_internal()
and gettime() when its caller had already dropped the lock.
event_pending() called gettime() without grabbing the lock.
Apparently some 64-bit platforms don't like it when you say
unsigned hash(void *p)
{
return (unsigned)p;
}
even if you really honestly don't want the high bits of p. Perhaps
they will tolerate it if I say the equivalent of
unsigned hash(void *p)
{
return (unsigned) (uintptr_t) p;
}
Our hash-table implementation stored a copy of the hash code in each
element. But as we were using it, all of our hash codes were
ridiculously easy to calculate: most of them were just a matter of a
load and a shift.
This patch lets ht-internal be built in either of two ways: one caches
the hash-code for each element, and one recalculates it each time it's
needed.
This patch also chooses a slightly better hash code for
event_debug_entry.
