* [PATCH 0/6] ds: object lifetime shortening
@ 2023-10-31 20:42 Eric Wong
2023-10-31 20:42 ` [PATCH 1/6] ds: next_tick: shorten object lifetimes Eric Wong
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
1 and 2 do the bulk of the work
3 and 4 are things I noticed when working on 2
5 is a fix while preparing for 6
6 is something I've wanted to do for a long time.
Anyways, the code reductions always make me happy.
Eric Wong (6):
ds: next_tick: shorten object lifetimes
ds: do not defer close
ds: move maxevents further down the stack
watch: simplify DirIdle object cleanup
pop3: use SSL_shutdown(3ssl) if appropriate
ds: make ->close behave like CORE::close
lib/PublicInbox/CidxComm.pm | 2 +-
lib/PublicInbox/CidxLogP.pm | 2 +-
lib/PublicInbox/DS.pm | 143 ++++++++++++------------------------
lib/PublicInbox/DSKQXS.pm | 3 +-
lib/PublicInbox/DSPoll.pm | 2 +-
lib/PublicInbox/Epoll.pm | 5 +-
lib/PublicInbox/POP3.pm | 2 +-
lib/PublicInbox/Select.pm | 2 +-
lib/PublicInbox/Watch.pm | 11 +--
t/ds-poll.t | 14 ++--
t/epoll.t | 4 +-
11 files changed, 71 insertions(+), 119 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/6] ds: next_tick: shorten object lifetimes
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
2023-10-31 20:42 ` [PATCH 2/6] ds: do not defer close Eric Wong
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
Drop reference counts ASAP in case it saves us some memory
sooner rather than later. This ought to give us more predictable
resource use and ensure OnDestroy callbacks fire sooner.
There's no need to use `local' to clobber the arrayref anymore,
either.
AFAIK, this doesn't fix any known bug, but more predictability
will make it easier to debug things going forward.
---
lib/PublicInbox/DS.pm | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 6041c6b5..708fe14d 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -136,16 +136,12 @@ sub _InitPoller () {
sub now () { clock_gettime(CLOCK_MONOTONIC) }
sub next_tick () {
- local $cur_runq = $nextq or return;
+ $cur_runq = $nextq or return;
$nextq = undef;
- for my $obj (@$cur_runq) {
+ while (my $obj = shift @$cur_runq) {
# avoid "ref" on blessed refs to workaround a Perl 5.16.3 leak:
# https://rt.perl.org/Public/Bug/Display.html?id=114340
- if (blessed($obj)) {
- $obj->event_step;
- } else {
- $obj->();
- }
+ blessed($obj) ? $obj->event_step : $obj->();
}
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/6] ds: do not defer close
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
2023-10-31 20:42 ` [PATCH 1/6] ds: next_tick: shorten object lifetimes Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
2023-10-31 20:42 ` [PATCH 3/6] ds: move maxevents further down the stack Eric Wong
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
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(-)
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)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/6] ds: move maxevents further down the stack
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
2023-10-31 20:42 ` [PATCH 1/6] ds: next_tick: shorten object lifetimes Eric Wong
2023-10-31 20:42 ` [PATCH 2/6] ds: do not defer close Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
2023-10-31 20:42 ` [PATCH 4/6] watch: simplify DirIdle object cleanup Eric Wong
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
The epoll implementation is the only one which respects the
limit (kevent would, but IO::KQueue does not). In any case,
I'm not a fan of the maxevents=1000 historical default since
it leads to fairness problems with shared non-blocking listeners
across multiple daemon workers.
---
lib/PublicInbox/DS.pm | 4 ++--
lib/PublicInbox/DSKQXS.pm | 3 ++-
lib/PublicInbox/DSPoll.pm | 2 +-
lib/PublicInbox/Epoll.pm | 5 ++++-
lib/PublicInbox/Select.pm | 2 +-
t/ds-poll.t | 14 +++++++-------
t/epoll.t | 4 ++--
7 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index b30e9db6..9e1f66c2 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -287,8 +287,8 @@ sub event_loop (;$$) {
do {
my $timeout = RunTimers();
- # get up to 1000 FDs representing events
- $Poller->ep_wait(1000, $timeout, \@active);
+ # grab whatever FDs are ready
+ $Poller->ep_wait($timeout, \@active);
# map all FDs to their associated Perl object
@active = @DescriptorMap{@active};
diff --git a/lib/PublicInbox/DSKQXS.pm b/lib/PublicInbox/DSKQXS.pm
index 8ef8ffb6..f84c196a 100644
--- a/lib/PublicInbox/DSKQXS.pm
+++ b/lib/PublicInbox/DSKQXS.pm
@@ -117,7 +117,8 @@ sub ep_del {
}
sub ep_wait {
- my ($self, $maxevents, $timeout_msec, $events) = @_;
+ my ($self, $timeout_msec, $events) = @_;
+ # n.b.: IO::KQueue is hard-coded to return up to 1000 events
@$events = eval { $self->{kq}->kevent($timeout_msec) };
if (my $err = $@) {
# workaround https://rt.cpan.org/Ticket/Display.html?id=116615
diff --git a/lib/PublicInbox/DSPoll.pm b/lib/PublicInbox/DSPoll.pm
index 0446df4c..b947f756 100644
--- a/lib/PublicInbox/DSPoll.pm
+++ b/lib/PublicInbox/DSPoll.pm
@@ -18,7 +18,7 @@ use Errno ();
sub new { bless {}, __PACKAGE__ } # fd => events
sub ep_wait {
- my ($self, $maxevents, $timeout_msec, $events) = @_;
+ my ($self, $timeout_msec, $events) = @_;
my (@pset, $n, $fd, $revents, $nval);
while (my ($fd, $events) = each %$self) {
my $pevents = $events & EPOLLIN ? POLLIN : 0;
diff --git a/lib/PublicInbox/Epoll.pm b/lib/PublicInbox/Epoll.pm
index d55c8535..7e0aa6e7 100644
--- a/lib/PublicInbox/Epoll.pm
+++ b/lib/PublicInbox/Epoll.pm
@@ -18,6 +18,9 @@ sub new {
sub ep_add { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_ADD, fileno($_[1]), $_[2]) }
sub ep_mod { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_MOD, fileno($_[1]), $_[2]) }
sub ep_del { epoll_ctl(fileno(${$_[0]}), EPOLL_CTL_DEL, fileno($_[1]), 0) }
-sub ep_wait { epoll_wait(fileno(${$_[0]}), @_[1, 2, 3]) }
+
+# n.b. maxevents=1000 is the historical default. maxevents=1 (yes, one)
+# is more fair under load with multiple worker processes sharing one listener
+sub ep_wait { epoll_wait(fileno(${$_[0]}), 1000, @_[1, 2]) }
1;
diff --git a/lib/PublicInbox/Select.pm b/lib/PublicInbox/Select.pm
index 5817c9ef..5cb7aff3 100644
--- a/lib/PublicInbox/Select.pm
+++ b/lib/PublicInbox/Select.pm
@@ -12,7 +12,7 @@ use PublicInbox::Syscall qw(EPOLLONESHOT EPOLLIN EPOLLOUT);
sub new { bless {}, __PACKAGE__ } # fd => events
sub ep_wait {
- my ($self, $maxevents, $msec, $events) = @_;
+ my ($self, $msec, $events) = @_;
my ($rvec, $wvec) = ('', ''); # we don't use EPOLLERR
while (my ($fd, $ev) = each %$self) {
vec($rvec, $fd, 1) = 1 if $ev & EPOLLIN;
diff --git a/t/ds-poll.t b/t/ds-poll.t
index 7a7e2bcf..321534bd 100644
--- a/t/ds-poll.t
+++ b/t/ds-poll.t
@@ -16,33 +16,33 @@ pipe($r, $w);
pipe($x, $y);
is($p->ep_add($r, EPOLLIN), 0, 'add EPOLLIN');
my $events = [];
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
is_deeply($events, [], 'no events set');
is($p->ep_add($w, EPOLLOUT|EPOLLONESHOT), 0, 'add EPOLLOUT|EPOLLONESHOT');
-$p->ep_wait(9, -1, $events);
+$p->ep_wait(-1, $events);
is(scalar(@$events), 1, 'got POLLOUT event');
is($events->[0], fileno($w), '$w ready');
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
is(scalar(@$events), 0, 'nothing ready after oneshot');
is_deeply($events, [], 'no events set after oneshot');
syswrite($w, '1') == 1 or die;
for my $t (0..1) {
- $p->ep_wait(9, $t, $events);
+ $p->ep_wait($t, $events);
is($events->[0], fileno($r), "level-trigger POLLIN ready #$t");
is(scalar(@$events), 1, "only event ready #$t");
}
syswrite($y, '1') == 1 or die;
is($p->ep_add($x, EPOLLIN|EPOLLONESHOT), 0, 'EPOLLIN|EPOLLONESHOT add');
-$p->ep_wait(9, -1, $events);
+$p->ep_wait(-1, $events);
is(scalar @$events, 2, 'epoll_wait has 2 ready');
my @fds = sort @$events;
my @exp = sort((fileno($r), fileno($x)));
is_deeply(\@fds, \@exp, 'got both ready FDs');
is($p->ep_del($r, 0), 0, 'EPOLL_CTL_DEL OK');
-$p->ep_wait(9, 0, $events);
+$p->ep_wait(0, $events);
is(scalar @$events, 0, 'nothing ready after EPOLL_CTL_DEL');
is($p->ep_add($r, EPOLLIN), 0, 're-add');
@@ -54,7 +54,7 @@ SKIP: {
my @w;
eval {
local $SIG{__WARN__} = sub { push @w, @_ };
- $p->ep_wait(9, 0, $events);
+ $p->ep_wait(0, $events);
};
ok($@, 'error detected from bad FD');
ok($!{EBADF}, 'EBADF errno set');
diff --git a/t/epoll.t b/t/epoll.t
index 54dc6f47..ab9ac22a 100644
--- a/t/epoll.t
+++ b/t/epoll.t
@@ -12,11 +12,11 @@ pipe(my $r, my $w);
is($ep->ep_add($w, EPOLLOUT), 0, 'epoll_ctl pipe EPOLLOUT');
my @events;
-$ep->ep_wait(100, 10000, \@events);
+$ep->ep_wait(10000, \@events);
is(scalar(@events), 1, 'got one event');
is($events[0], fileno($w), 'got expected FD');
close $w;
-$ep->ep_wait(100, 0, \@events);
+$ep->ep_wait(0, \@events);
is(scalar(@events), 0, 'epoll_wait timeout');
done_testing;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/6] watch: simplify DirIdle object cleanup
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
` (2 preceding siblings ...)
2023-10-31 20:42 ` [PATCH 3/6] ds: move maxevents further down the stack Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
2023-10-31 20:42 ` [PATCH 5/6] pop3: use SSL_shutdown(3ssl) if appropriate Eric Wong
2023-10-31 20:42 ` [PATCH 6/6] ds: make ->close behave like CORE::close Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
There's no need to waste time nor reach into DS internals to
map FDs to Perl objects, here. LEI.pm has never had to deal
with integer FDs for DirIdle, either.
---
lib/PublicInbox/Watch.pm | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 41b77dc1..c2b1312a 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -256,9 +256,8 @@ sub quit { # may be called in IMAP/NNTP children
%{$self->{opendirs}} = ();
_done_for_now($self);
quit_done($self);
- if (defined(my $fd = delete $self->{dir_idle_fd})) {
- my $di = $PublicInbox::DS::DescriptorMap{$fd};
- $di->close if $di && $di->can('add_watches');
+ if (my $dir_idle = delete $self->{dir_idle}) {
+ $dir_idle->close if $dir_idle;
}
if (my $idle_mic = delete $self->{idle_mic}) { # IMAP child
return unless $idle_mic->IsConnected && $idle_mic->Socket;
@@ -283,8 +282,7 @@ sub watch_fs_init ($) {
};
require PublicInbox::DirIdle;
# inotify_create + EPOLL_CTL_ADD
- my $dir_idle = PublicInbox::DirIdle->new($cb);
- $self->{dir_idle_fd} = fileno($dir_idle->{sock}) if $dir_idle->{sock};
+ my $dir_idle = $self->{dir_idle} = PublicInbox::DirIdle->new($cb);
$dir_idle->add_watches([keys %{$self->{mdmap}}]);
}
@@ -383,8 +381,7 @@ sub watch_imap_idle_1 ($$$) {
sub watch_atfork_child ($) {
my ($self) = @_;
- delete $self->{pids};
- delete $self->{opendirs};
+ delete @$self{qw(dir_idle pids opendirs)};
my $sig = delete $self->{sig};
$sig->{CHLD} = $sig->{HUP} = $sig->{USR1} = 'DEFAULT';
# TERM/QUIT/INT call ->quit, which works in both parent+child
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/6] pop3: use SSL_shutdown(3ssl) if appropriate
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
` (3 preceding siblings ...)
2023-10-31 20:42 ` [PATCH 4/6] watch: simplify DirIdle object cleanup Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
2023-10-31 20:42 ` [PATCH 6/6] ds: make ->close behave like CORE::close Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
This allows us support SSL session caching + reuse in the future.
---
lib/PublicInbox/POP3.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/PublicInbox/POP3.pm b/lib/PublicInbox/POP3.pm
index 6d24b17c..06772069 100644
--- a/lib/PublicInbox/POP3.pm
+++ b/lib/PublicInbox/POP3.pm
@@ -370,7 +370,7 @@ UPDATE users SET last_seen = ? WHERE user_id = ?
$self->{pop3d}->unlock_mailbox($self);
}
$self->write(\"+OK public-inbox POP3 server signing off\r\n");
- $self->close;
+ $self->shutdn;
undef;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/6] ds: make ->close behave like CORE::close
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
` (4 preceding siblings ...)
2023-10-31 20:42 ` [PATCH 5/6] pop3: use SSL_shutdown(3ssl) if appropriate Eric Wong
@ 2023-10-31 20:42 ` Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2023-10-31 20:42 UTC (permalink / raw)
To: meta
Matching existing Perl IO semantics seems like a good idea to
reduce confusion in the future.
We'll also fix some outdated comments and update indentation
to match the rest of our code base since we're far detached from
Danga::Socket at this point.
---
lib/PublicInbox/CidxComm.pm | 2 +-
lib/PublicInbox/CidxLogP.pm | 2 +-
lib/PublicInbox/DS.pm | 84 ++++++++++++++-----------------------
3 files changed, 33 insertions(+), 55 deletions(-)
diff --git a/lib/PublicInbox/CidxComm.pm b/lib/PublicInbox/CidxComm.pm
index fb7be0aa..c7ab3c10 100644
--- a/lib/PublicInbox/CidxComm.pm
+++ b/lib/PublicInbox/CidxComm.pm
@@ -21,7 +21,7 @@ sub new {
sub event_step {
my ($self) = @_;
my $rd = $self->{sock} // return warn('BUG?: no {sock}');
- $self->close; # PublicInbox::DS::close, deferred, so $sock is usable
+ $self->close; # EPOLL_CTL_DEL
delete($self->{cidx})->cidx_read_comm($rd);
}
diff --git a/lib/PublicInbox/CidxLogP.pm b/lib/PublicInbox/CidxLogP.pm
index ac4c1b37..5ea675bf 100644
--- a/lib/PublicInbox/CidxLogP.pm
+++ b/lib/PublicInbox/CidxLogP.pm
@@ -21,7 +21,7 @@ sub new {
sub event_step {
my ($self) = @_;
my $rd = $self->{sock} // return warn('BUG?: no {sock}');
- $self->close; # PublicInbox::DS::close, deferred, so $sock is usable
+ $self->close; # EPOLL_CTL_DEL
delete($self->{cidx})->cidx_read_log_p($self, $rd);
}
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 9e1f66c2..ca1f0f81 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -354,37 +354,21 @@ sub greet {
$self;
}
-#####################################################################
-### I N S T A N C E M E T H O D S
-#####################################################################
-
sub requeue ($) { push @$nextq, $_[0] } # autovivifies
-=head2 C<< $obj->close >>
-
-Close the socket.
-
-=cut
+# drop the IO::Handle ref, true if successful, false if not (or already dropped)
+# (this is closer to CORE::close than Danga::Socket::close)
sub close {
- my ($self) = @_;
- my $sock = delete $self->{sock} or return;
-
- # we need to flush our write buffer, as there may
- # be self-referential closures (sub { $client->close })
- # preventing the object from being destroyed
- delete $self->{wbuf};
-
- delete $DescriptorMap{fileno($sock)};
+ my ($self) = @_;
+ my $sock = delete $self->{sock} or return;
- # 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..
+ # we need to clear our write buffer, as there may
+ # be self-referential closures (sub { $client->close })
+ # preventing the object from being destroyed
+ delete $self->{wbuf};
+ delete $DescriptorMap{fileno($sock)};
- # 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;
+ !$Poller->ep_del($sock); # stop getting notifications
}
# portable, non-thread-safe sendfile emulation (no pread, yet)
@@ -430,8 +414,7 @@ next_buf:
shift @$wbuf;
goto next_buf;
}
- } elsif ($! == EAGAIN) {
- my $ev = epbit($sock, EPOLLOUT) or return $self->close;
+ } elsif ($! == EAGAIN && (my $ev = epbit($sock, EPOLLOUT))) {
epwait($sock, $ev | EPOLLONESHOT);
return 0;
} else {
@@ -461,28 +444,28 @@ sub rbuf_idle ($$) {
}
}
+# returns true if bytes are read, false otherwise
sub do_read ($$$;$) {
- my ($self, $rbuf, $len, $off) = @_;
- my $r = sysread(my $sock = $self->{sock}, $$rbuf, $len, $off // 0);
- return ($r == 0 ? $self->close : $r) if defined $r;
- # common for clients to break connections without warning,
- # would be too noisy to log here:
- if ($! == EAGAIN) {
- my $ev = epbit($sock, EPOLLIN) or return $self->close;
- epwait($sock, $ev | EPOLLONESHOT);
- rbuf_idle($self, $rbuf);
- 0;
- } else {
- $self->close;
- }
+ my ($self, $rbuf, $len, $off) = @_;
+ my ($ev, $r, $s);
+ $r = sysread($s = $self->{sock}, $$rbuf, $len, $off // 0) and return $r;
+
+ if (!defined($r) && $! == EAGAIN && ($ev = epbit($s, EPOLLIN))) {
+ epwait($s, $ev | EPOLLONESHOT);
+ rbuf_idle($self, $rbuf);
+ } else {
+ $self->close;
+ }
+ undef;
}
# drop the socket if we hit unrecoverable errors on our system which
# require BOFH attention: ENOSPC, EFBIG, EIO, EMFILE, ENFILE...
sub drop {
- my $self = shift;
- carp(@_);
- $self->close;
+ my $self = shift;
+ carp(@_);
+ $self->close;
+ undef;
}
sub tmpio ($$$) {
@@ -603,7 +586,7 @@ sub accept_tls_step ($) {
0;
}
-# return true if complete, false if incomplete (or failure)
+# return value irrelevant
sub shutdn_tls_step ($) {
my ($self) = @_;
my $sock = $self->{sock} or return;
@@ -612,19 +595,14 @@ sub shutdn_tls_step ($) {
my $ev = PublicInbox::TLS::epollbit() or return $self->close;
epwait($sock, $ev | EPOLLONESHOT);
unshift(@{$self->{wbuf}}, \&shutdn_tls_step); # autovivifies
- 0;
}
# don't bother with shutdown($sock, 2), we don't fork+exec w/o CLOEXEC
# or fork w/o exec, so no inadvertent socket sharing
sub shutdn ($) {
- my ($self) = @_;
- my $sock = $self->{sock} or return;
- if ($sock->can('stop_SSL')) {
- shutdn_tls_step($self);
- } else {
- $self->close;
- }
+ my ($self) = @_;
+ my $sock = $self->{sock} or return;
+ $sock->can('stop_SSL') ? shutdn_tls_step($self) : $self->close;
}
sub dflush {} # overridden by DSdeflate
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-31 20:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-31 20:42 [PATCH 0/6] ds: object lifetime shortening Eric Wong
2023-10-31 20:42 ` [PATCH 1/6] ds: next_tick: shorten object lifetimes Eric Wong
2023-10-31 20:42 ` [PATCH 2/6] ds: do not defer close Eric Wong
2023-10-31 20:42 ` [PATCH 3/6] ds: move maxevents further down the stack Eric Wong
2023-10-31 20:42 ` [PATCH 4/6] watch: simplify DirIdle object cleanup Eric Wong
2023-10-31 20:42 ` [PATCH 5/6] pop3: use SSL_shutdown(3ssl) if appropriate Eric Wong
2023-10-31 20:42 ` [PATCH 6/6] ds: make ->close behave like CORE::close Eric Wong
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).