From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 095321F4BC for ; Sat, 29 Jun 2019 19:59:53 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 06/11] ds: consolidate IO::Socket::SSL checks Date: Sat, 29 Jun 2019 19:59:46 +0000 Message-Id: <20190629195951.32160-7-e@80x24.org> In-Reply-To: <20190629195951.32160-1-e@80x24.org> References: <20190629195951.32160-1-e@80x24.org> List-Id: We need to be careful about handling EAGAIN on write(2) failures deal with SSL_WANT_READ vs SSL_WANT_WRITE as appropriate. --- lib/PublicInbox/DS.pm | 48 +++++++++++++++++++++++++----------------------- lib/PublicInbox/NNTP.pm | 3 ++- lib/PublicInbox/TLS.pm | 9 +++------ 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index 6cd527e2..b2f59983 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -388,6 +388,10 @@ sub psendfile ($$$) { $written; } +sub epbit ($$) { # (sock, default) + ref($_[0]) eq 'IO::Socket::SSL' ? PublicInbox::TLS::epollbit() : $_[1]; +} + # returns 1 if done, 0 if incomplete sub flush_write ($) { my ($self) = @_; @@ -406,8 +410,8 @@ next_buf: goto next_buf; } } elsif ($! == EAGAIN) { + epwait($sock, epbit($sock, EPOLLOUT) | EPOLLONESHOT); $self->{wbuf_off} = $off; - watch($self, EPOLLOUT|EPOLLONESHOT); return 0; } else { return $self->close; @@ -438,17 +442,13 @@ sub rbuf_idle ($$) { sub do_read ($$$;$) { my ($self, $rbuf, $len, $off) = @_; - my $r = sysread($self->{sock}, $$rbuf, $len, $off // 0); + 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 (ref($self) eq 'IO::Socket::SSL') { - my $ev = PublicInbox::TLS::epollbit() or return $self->close; + if ($! == EAGAIN) { + epwait($sock, epbit($sock, EPOLLIN) | EPOLLONESHOT); rbuf_idle($self, $rbuf); - watch($self, $ev | EPOLLONESHOT); - } elsif ($! == EAGAIN) { - rbuf_idle($self, $rbuf); - watch($self, EPOLLIN | EPOLLONESHOT); } else { $self->close; } @@ -525,17 +525,20 @@ sub write { if (defined $written) { return 1 if $written == $to_write; + requeue($self); # runs: event_step -> flush_write } elsif ($! == EAGAIN) { + epwait($sock, epbit($sock, EPOLLOUT) | EPOLLONESHOT); $written = 0; } else { return $self->close; } + + # deal with EAGAIN or partial write: my $tmpio = tmpio($self, $bref, $written) or return 0; # wbuf may be an empty array if we're being called inside # ->flush_write via CODE bref: push @{$self->{wbuf} ||= []}, $tmpio; - watch($self, EPOLLOUT|EPOLLONESHOT); return 0; } } @@ -554,32 +557,34 @@ sub msg_more ($$) { # queue up the unwritten substring: my $tmpio = tmpio($self, \($_[1]), $n) or return 0; $self->{wbuf} = [ $tmpio ]; - watch($self, EPOLLOUT|EPOLLONESHOT); + epwait($sock, EPOLLOUT|EPOLLONESHOT); return 0; } } $self->write(\($_[1])); } -sub watch ($$) { - my ($self, $ev) = @_; - my $sock = $self->{sock} or return; +sub epwait ($$) { + my ($sock, $ev) = @_; epoll_ctl($Epoll, EPOLL_CTL_MOD, fileno($sock), $ev) and confess("EPOLL_CTL_MOD $!"); 0; } +sub watch ($$) { + my ($self, $ev) = @_; + my $sock = $self->{sock} or return; + epwait($sock, $ev); +} + # return true if complete, false if incomplete (or failure) sub accept_tls_step ($) { my ($self) = @_; my $sock = $self->{sock} or return; return 1 if $sock->accept_SSL; return $self->close if $! != EAGAIN; - if (my $ev = PublicInbox::TLS::epollbit()) { - unshift @{$self->{wbuf} ||= []}, \&accept_tls_step; - return watch($self, $ev | EPOLLONESHOT); - } - drop($self, 'BUG? EAGAIN but '.PublicInbox::TLS::err()); + epwait($sock, PublicInbox::TLS::epollbit() | EPOLLONESHOT); + unshift @{$self->{wbuf} ||= []}, \&accept_tls_step; } sub shutdn_tls_step ($) { @@ -587,11 +592,8 @@ sub shutdn_tls_step ($) { my $sock = $self->{sock} or return; return $self->close if $sock->stop_SSL(SSL_fast_shutdown => 1); return $self->close if $! != EAGAIN; - if (my $ev = PublicInbox::TLS::epollbit()) { - unshift @{$self->{wbuf} ||= []}, \&shutdn_tls_step; - return watch($self, $ev | EPOLLONESHOT); - } - drop($self, 'BUG? EAGAIN but '.PublicInbox::TLS::err()); + epwait($sock, PublicInbox::TLS::epollbit() | EPOLLONESHOT); + unshift @{$self->{wbuf} ||= []}, \&shutdn_tls_step; } # don't bother with shutdown($sock, 2), we don't fork+exec w/o CLOEXEC diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 9973fcaf..82762b1a 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -75,7 +75,8 @@ sub new ($$$) { my $ev = EPOLLIN; my $wbuf; if (ref($sock) eq 'IO::Socket::SSL' && !$sock->accept_SSL) { - $ev = PublicInbox::TLS::epollbit() or return CORE::close($sock); + return CORE::close($sock) if $! != EAGAIN; + $ev = PublicInbox::TLS::epollbit(); $wbuf = [ \&PublicInbox::DS::accept_tls_step, \&greet ]; } $self->SUPER::new($sock, $ev | EPOLLONESHOT); diff --git a/lib/PublicInbox/TLS.pm b/lib/PublicInbox/TLS.pm index 576c11d7..0b9a55df 100644 --- a/lib/PublicInbox/TLS.pm +++ b/lib/PublicInbox/TLS.pm @@ -13,12 +13,9 @@ sub err () { $SSL_ERROR } # returns the EPOLL event bit which matches the existing SSL error sub epollbit () { - if ($! == EAGAIN) { - return EPOLLIN if $SSL_ERROR == SSL_WANT_READ; - return EPOLLOUT if $SSL_ERROR == SSL_WANT_WRITE; - die "unexpected SSL error: $SSL_ERROR"; - } - 0; + return EPOLLIN if $SSL_ERROR == SSL_WANT_READ; + return EPOLLOUT if $SSL_ERROR == SSL_WANT_WRITE; + die "unexpected SSL error: $SSL_ERROR"; } 1; -- EW