From 4d2f3651bde2f2c61b78973df56b6e6ee37a6dce Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 31 Oct 2023 20:42:51 +0000 Subject: ds: do not defer close We can map all integer FDs to Perl objects once ->ep_wait returns, so there's no need to play tricks elsewhere to ensure FDs can be mapped to objects within the same event loop iteration. --- lib/PublicInbox/DS.pm | 67 +++++++++++++++++---------------------------------- 1 file changed, 22 insertions(+), 45 deletions(-) (limited to 'lib/PublicInbox/DS.pm') diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index 708fe14d..b30e9db6 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -39,7 +39,7 @@ our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer); my $nextq; # queue for next_tick my $reap_armed; -my $ToClose; # sockets to close when event loop is done +my @active; # FDs (or objs) returned by epoll/kqueue our (%AWAIT_PIDS, # pid => [ $callback, @args ] $cur_runq, # only set inside next_tick %DescriptorMap, # fd (num) -> PublicInbox::DS object @@ -81,10 +81,11 @@ sub Reset { # we may be called from an *atfork_child inside next_tick: @$cur_runq = () if $cur_runq; - $nextq = $ToClose = undef; # may call ep_del + @active = (); + $nextq = undef; # may call ep_del %AWAIT_PIDS = (); } while (@Timers || $nextq || keys(%AWAIT_PIDS) || - $ToClose || keys(%DescriptorMap) || + @active || keys(%DescriptorMap) || @post_loop_do || keys(%UniqTimer) || scalar(@{$cur_runq // []})); # do not vivify cur_runq @@ -149,7 +150,7 @@ sub next_tick () { sub RunTimers { next_tick(); - return (($nextq || $ToClose) ? 0 : $loop_timeout) unless @Timers; + return ($nextq ? 0 : $loop_timeout) unless @Timers; my $now = now(); @@ -161,7 +162,7 @@ sub RunTimers { } # timers may enqueue into nextq: - return 0 if ($nextq || $ToClose); + return 0 if $nextq; return $loop_timeout unless @Timers; @@ -236,18 +237,6 @@ sub close_non_busy () { # for pushed-back data, and close pending connections. returns 1 # if event loop should continue, or 0 to shut it all down. sub PostEventLoop () { - # now we can close sockets that wanted to close during our event - # processing. (we didn't want to close them during the loop, as we - # didn't want fd numbers being reused and confused during the event - # loop) - if (my $close_now = $ToClose) { - $ToClose = undef; # will be autovivified on push - @$close_now = map { fileno($_) } @$close_now; - - # ->DESTROY methods may populate ToClose - delete @DescriptorMap{@$close_now}; - } - # by default we keep running, unless a postloop callback cancels it @post_loop_do ? $post_loop_do[0]->(@post_loop_do[1..$#post_loop_do]) : 1 } @@ -295,21 +284,16 @@ sub event_loop (;$$) { } $_[0] = $sigfd = $sig = undef; # $_[0] == sig local $in_loop = 1; - my @events; do { my $timeout = RunTimers(); - # get up to 1000 events - $Poller->ep_wait(1000, $timeout, \@events); - for my $fd (@events) { - # it's possible epoll_wait returned many events, - # including some at the end that ones in the front - # triggered unregister-interest actions. if we can't - # find the %sock entry, it's because we're no longer - # interested in that event. - - # guard stack-not-refcounted w/ Carp + @DB::args - my $obj = $DescriptorMap{$fd}; + # get up to 1000 FDs representing events + $Poller->ep_wait(1000, $timeout, \@active); + + # map all FDs to their associated Perl object + @active = @DescriptorMap{@active}; + + while (my $obj = shift @active) { $obj->event_step; } } while (PostEventLoop()); @@ -390,24 +374,17 @@ sub close { # preventing the object from being destroyed delete $self->{wbuf}; - # if we're using epoll, we have to remove this from our epoll fd so we stop getting - # notifications about it - $Poller->ep_del($sock) and croak("EPOLL_CTL_DEL($self/$sock): $!"); - - # we explicitly don't delete from DescriptorMap here until we - # actually close the socket, as we might be in the middle of - # processing an epoll_wait/etc that returned hundreds of fds, one - # of which is not yet processed and is what we're closing. if we - # keep it in DescriptorMap, then the event harnesses can just - # looked at $pob->{sock} == undef and ignore it. but if it's an - # un-accounted for fd, then it (understandably) freak out a bit - # and emit warnings, thinking their state got off. + delete $DescriptorMap{fileno($sock)}; - # defer closing the actual socket until the event loop is done - # processing this round of events. (otherwise we might reuse fds) - push @$ToClose, $sock; # autovivifies $ToClose + # if we're using epoll, we have to remove this from our epoll fd so we + # stop getting notifications about it + $Poller->ep_del($sock) and croak("EPOLL_CTL_DEL($self/$sock): $!"); + # let refcount drop to zero.. - return 0; + # FIXME this is the opposite of CORE::close return value + # TODO: callers need to be updated to expect true on success like + # CORE::close (and false otherwise) + 0; } # portable, non-thread-safe sendfile emulation (no pread, yet) -- cgit v1.2.3-24-ge0c7