From: Eric Wong <e@yhbt.net>
To: meta@public-inbox.org
Subject: [PATCH 05/11] ds: guard ToClose against DESTROY side-effects
Date: Sun, 12 Jan 2020 21:17:50 +0000 [thread overview]
Message-ID: <20200112211756.23100-6-e@yhbt.net> (raw)
In-Reply-To: <20200112211756.23100-1-e@yhbt.net>
This does not affect our current code, but theoretically a
DESTROY callback could call PublicInbox::DS::close to enqueue
elements into the ToClose array. So take a similar strategy as
we do with other queues (e.g. $nextq) by swapping references to
arrays, rather than operating on the array itself.
Since close operations are relatively rare, we can rely on
auto-vivification via "push" ops to create the array on an
as-needed basis.
Since we're in the area, clean up the PostLoopCallback
invocation to use the ternary operator rather than a confusing
(to me) combination of statements.
Finally, add a prototype to strengthen compile-time checking,
and move it in front of our only caller to make use of
the prototype.
---
lib/PublicInbox/DS.pm | 52 ++++++++++++++++++++-----------------------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 57c945f5..52b11386 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -44,11 +44,11 @@ my $later_queue; # callbacks
my $EXPMAP; # fd -> [ idle_time, $self ]
our $EXPTIME = 180; # 3 minutes
my ($later_timer, $reap_timer, $exp_timer);
+my $ToClose; # sockets to close when event loop is done
our (
%DescriptorMap, # fd (num) -> PublicInbox::DS object
$Epoll, # Global epoll fd (or DSKQXS ref)
$_io, # IO::Handle for Epoll
- @ToClose, # sockets to close when event loop is done
$PostLoopCallback, # subref to call at the end of each loop, if defined (global)
@@ -75,8 +75,7 @@ sub Reset {
$WaitPids = [];
$later_queue = [];
$EXPMAP = {};
- $reap_timer = $later_timer = $exp_timer = undef;
- @ToClose = ();
+ $ToClose = $reap_timer = $later_timer = $exp_timer = undef;
$LoopTimeout = -1; # no timeout by default
@Timers = ();
@@ -197,7 +196,7 @@ sub next_tick () {
sub RunTimers {
next_tick();
- return ((@$nextq || @ToClose) ? 0 : $LoopTimeout) unless @Timers;
+ return ((@$nextq || $ToClose) ? 0 : $LoopTimeout) unless @Timers;
my $now = now();
@@ -208,7 +207,7 @@ sub RunTimers {
}
# timers may enqueue into nextq:
- return 0 if (@$nextq || @ToClose);
+ return 0 if (@$nextq || $ToClose);
return $LoopTimeout unless @Timers;
@@ -254,6 +253,25 @@ sub enqueue_reap ($) { push @$nextq, \&reap_pids };
sub in_loop () { $in_loop }
+# Internal function: run the post-event callback, send read events
+# 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
+ # ->DESTROY methods may populate ToClose
+ delete($DescriptorMap{fileno($_)}) for @$close_now;
+ # let refcounting drop everything in $close_now at once
+ }
+
+ # by default we keep running, unless a postloop callback cancels it
+ $PostLoopCallback ? $PostLoopCallback->(\%DescriptorMap) : 1;
+}
+
sub EpollEventLoop {
local $in_loop = 1;
do {
@@ -292,28 +310,6 @@ sub SetPostLoopCallback {
$PostLoopCallback = (defined $ref && ref $ref eq 'CODE') ? $ref : undef;
}
-# Internal function: run the post-event callback, send read events
-# 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)
- delete($DescriptorMap{fileno($_)}) for @ToClose;
- @ToClose = (); # let refcounting drop everything all at once
-
- # by default we keep running, unless a postloop callback (either per-object
- # or global) cancels it
- my $keep_running = 1;
-
- # now we're at the very end, call callback if defined
- if (defined $PostLoopCallback) {
- $keep_running &&= $PostLoopCallback->(\%DescriptorMap);
- }
-
- return $keep_running;
-}
-
#####################################################################
### PublicInbox::DS-the-object code
#####################################################################
@@ -390,7 +386,7 @@ sub close {
# defer closing the actual socket until the event loop is done
# processing this round of events. (otherwise we might reuse fds)
- push @ToClose, $sock;
+ push @$ToClose, $sock; # autovivifies $ToClose
return 0;
}
next prev parent reply other threads:[~2020-01-12 21:17 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-12 21:17 [PATCH 00/11] ds: various cleanups and fixes Eric Wong
2020-01-12 21:17 ` [PATCH 01/11] ds: new: avoid redundant check, make clobbering fatal Eric Wong
2020-01-12 21:17 ` [PATCH 02/11] ds: add_timer: rename from AddTimer, remove a parameter Eric Wong
2020-01-12 21:17 ` [PATCH 03/11] ds: add an in_loop() function for Inbox.pm use Eric Wong
2020-01-12 21:17 ` [PATCH 04/11] ds: remove Timer->cancel and Timer class+bless Eric Wong
2020-01-12 21:17 ` Eric Wong [this message]
2020-01-12 21:17 ` [PATCH 06/11] ds|http|nntp: simplify {wbuf} population Eric Wong
2020-01-12 21:17 ` [PATCH 07/11] ds: rely on autovivification for nextq Eric Wong
2020-01-12 21:17 ` [PATCH 08/11] ds: rely on autovivication for waitpid bits Eric Wong
2020-01-12 21:17 ` [PATCH 09/11] ds: rely on autovivification for $later_queue Eric Wong
2020-01-12 21:17 ` [PATCH 10/11] ds: flatten $EXPMAP, delete entries on close Eric Wong
2020-01-12 21:17 ` [PATCH 11/11] sigfd: simplify loop and improve documentation Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://public-inbox.org/README
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200112211756.23100-6-e@yhbt.net \
--to=e@yhbt.net \
--cc=meta@public-inbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).