* [PATCH 1/2] send and receive all 3 FDs at once
2021-01-03 11:24 7% ` [PATCH 0/2] fix race from stdout buffering in FD pass exit Eric Wong
@ 2021-01-03 11:24 6% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2021-01-03 11:24 UTC (permalink / raw)
To: meta
We'll always be transferring stdin, stdout, and stderr together
for lei. Perhaps I lack imagination or foresight, but I can't
think of a reason to send more or less FDs.
---
lib/PublicInbox/LEI.pm | 27 ++++++++++----------
lib/PublicInbox/Spawn.pm | 53 ++++++++++++++++++++++++++--------------
script/lei | 6 ++---
t/lei.t | 2 +-
t/spawn.t | 33 ++++++++++++++++---------
5 files changed, 74 insertions(+), 47 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 6f98c934..3ad5e01a 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -25,7 +25,7 @@ use Text::Wrap qw(wrap);
use File::Path qw(mkpath);
use File::Spec;
our $quit = \&CORE::exit;
-my $recv_fd;
+my $recv_3fds;
my $GLP = Getopt::Long::Parser->new;
$GLP->configure(qw(gnu_getopt no_ignore_case auto_abbrev));
my $GLP_PASS = Getopt::Long::Parser->new;
@@ -614,25 +614,26 @@ sub accept_dispatch { # Listener {post_accept} callback
my $self = bless { sock => $sock }, __PACKAGE__;
vec(my $rin = '', fileno($sock), 1) = 1;
# `say $sock' triggers "die" in lei(1)
- for my $i (0..2) {
- if (select(my $rout = $rin, undef, undef, 1)) {
- my $fd = $recv_fd->(fileno($sock));
- if ($fd >= 0) {
- my $rdr = ($fd == 0 ? '<&=' : '>&=');
+ if (select(my $rout = $rin, undef, undef, 1)) {
+ my @fds = $recv_3fds->(fileno($sock));
+ if (scalar(@fds) == 3) {
+ my $i = 0;
+ for my $rdr (qw(<&= >&= >&=)) {
+ my $fd = shift(@fds);
if (open(my $fh, $rdr, $fd)) {
- $self->{$i} = $fh;
- } else {
+ $self->{$i++} = $fh;
+ } else {
say $sock "open($rdr$fd) (FD=$i): $!";
return;
}
- } else {
- say $sock "recv FD=$i: $!";
- return;
}
} else {
- say $sock "timed out waiting to recv FD=$i";
+ say $sock "recv_3fds failed: $!";
return;
}
+ } else {
+ say $sock "timed out waiting to recv FDs";
+ return;
}
# $ARGV_STR = join("]\0[", @ARGV);
# $ENV_STR = join('', map { "$_=$ENV{$_}\0" } keys %ENV);
@@ -672,7 +673,7 @@ sub lazy_start {
my $dev_ino_expect = pack('dd', $st[0], $st[1]); # dev+ino
pipe(my ($eof_r, $eof_w)) or die "pipe: $!";
my $oldset = PublicInbox::Sigfd::block_signals();
- $recv_fd = PublicInbox::Spawn->can('recv_fd') or die
+ $recv_3fds = PublicInbox::Spawn->can('recv_3fds') or die
"Inline::C not installed/configured or IO::FDPass missing\n";
require PublicInbox::Listener;
require PublicInbox::EOFpipe;
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 4ca94b9f..61e95433 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -207,56 +207,67 @@ my $fdpass = <<'FDPASS';
#include <sys/socket.h>
#if defined(CMSG_SPACE) && defined(CMSG_LEN)
+struct my_3fds { int fds[3]; };
union my_cmsg {
struct cmsghdr hdr;
- char pad[sizeof(struct cmsghdr)+8+sizeof(int)+8];
+ char pad[sizeof(struct cmsghdr)+ 8 + sizeof(struct my_3fds) + 8];
};
-int send_fd(int sockfd, int fd)
+int send_3fds(int sockfd, int infd, int outfd, int errfd)
{
struct msghdr msg = { 0 };
struct iovec iov;
union my_cmsg cmsg = { 0 };
+ int *fdp;
+ size_t i;
- iov.iov_base = &msg.msg_namelen;
+ iov.iov_base = &msg.msg_namelen; /* whatever */
iov.iov_len = 1;
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
msg.msg_control = &cmsg.hdr;
- msg.msg_controllen = CMSG_SPACE(sizeof(int));
+ msg.msg_controllen = CMSG_SPACE(sizeof(struct my_3fds));
cmsg.hdr.cmsg_level = SOL_SOCKET;
cmsg.hdr.cmsg_type = SCM_RIGHTS;
- cmsg.hdr.cmsg_len = CMSG_LEN(sizeof(int));
- *(int *)CMSG_DATA(&cmsg.hdr) = fd;
-
+ cmsg.hdr.cmsg_len = CMSG_LEN(sizeof(struct my_3fds));
+ fdp = (int *)CMSG_DATA(&cmsg.hdr);
+ *fdp++ = infd;
+ *fdp++ = outfd;
+ *fdp++ = errfd;
return sendmsg(sockfd, &msg, 0) >= 0;
}
-int recv_fd(int sockfd)
+void recv_3fds(int sockfd)
{
union my_cmsg cmsg = { 0 };
struct msghdr msg = { 0 };
struct iovec iov;
- int fd = -1;
+ size_t i;
+ Inline_Stack_Vars;
- iov.iov_base = &msg.msg_namelen;
+ iov.iov_base = &msg.msg_namelen; /* whatever */
iov.iov_len = 1;
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
msg.msg_control = &cmsg.hdr;
- msg.msg_controllen = CMSG_SPACE(sizeof(int));
+ msg.msg_controllen = CMSG_SPACE(sizeof(struct my_3fds));
if (recvmsg(sockfd, &msg, 0) <= 0)
- return -1;
+ return;
errno = EDOM;
+ Inline_Stack_Reset;
if (cmsg.hdr.cmsg_level == SOL_SOCKET &&
cmsg.hdr.cmsg_type == SCM_RIGHTS &&
- cmsg.hdr.cmsg_len == CMSG_LEN(sizeof(int)))
- fd = *(int *)CMSG_DATA(&cmsg.hdr);
+ cmsg.hdr.cmsg_len == CMSG_LEN(sizeof(struct my_3fds))) {
+ int *fdp = (int *)CMSG_DATA(&cmsg.hdr);
+ size_t i;
- return fd;
+ for (i = 0; i < 3; i++)
+ Inline_Stack_Push(sv_2mortal(newSViv(*fdp++)));
+ }
+ Inline_Stack_Done;
}
#endif /* defined(CMSG_SPACE) && defined(CMSG_LEN) */
FDPASS
@@ -275,7 +286,8 @@ if (defined $vfork_spawn) {
my $f = "$inline_dir/.public-inbox.lock";
open my $fh, '>', $f or die "failed to open $f: $!\n";
flock($fh, LOCK_EX) or die "LOCK_EX failed on $f: $!\n";
- eval 'use Inline C => $vfork_spawn . $fdpass . $set_nodatacow';
+ eval 'use Inline C => $vfork_spawn.$fdpass.$set_nodatacow';
+ # . ', BUILD_NOISY => 1';
my $err = $@;
my $ndc_err;
if ($err && $set_nodatacow) { # missing Linux kernel headers
@@ -303,12 +315,15 @@ unless ($set_nodatacow) {
*nodatacow_fd = \&PublicInbox::NDC_PP::nodatacow_fd;
*nodatacow_dir = \&PublicInbox::NDC_PP::nodatacow_dir;
}
-unless (__PACKAGE__->can('recv_fd')) {
+unless (__PACKAGE__->can('recv_3fds')) {
eval { # try the XS IO::FDPass package
require IO::FDPass;
no warnings 'once';
- *recv_fd = \&IO::FDPass::recv;
- *send_fd = \&IO::FDPass::send;
+ *recv_3fds = sub { map { IO::FDPass::recv($_[0]) } (0..2) };
+ *send_3fds = sub ($$$$) {
+ my $sockfd = shift;
+ IO::FDPass::send($sockfd, shift) for (0..2);
+ };
};
}
diff --git a/script/lei b/script/lei
index 67e8b8b0..029881f8 100755
--- a/script/lei
+++ b/script/lei
@@ -4,10 +4,10 @@
use strict;
use v5.10.1;
use Socket qw(AF_UNIX SOCK_STREAM pack_sockaddr_un);
-my $send_fd;
+my $send_3fds;
if (my ($sock, $pwd) = eval {
require PublicInbox::Spawn;
- $send_fd = PublicInbox::Spawn->can('send_fd') or die
+ $send_3fds = PublicInbox::Spawn->can('send_3fds') or die
"Inline::C not installed/configured or IO::FDPass missing\n";
my $path = do {
my $runtime_dir = ($ENV{XDG_RUNTIME_DIR} // '') . '/lei';
@@ -60,7 +60,7 @@ Falling back to (slow) one-shot mode
$buf .= "\0\0";
select $sock;
$| = 1; # unbuffer selected $sock
- $send_fd->(fileno($sock), $_) for (0..2);
+ $send_3fds->(fileno($sock), 0, 1, 2);
print $sock $buf or die "print(sock, buf): $!";
while ($buf = <$sock>) {
$buf =~ /\Aexit=([0-9]+)\n\z/ and exit($1 + 0);
diff --git a/t/lei.t b/t/lei.t
index 662fc545..42c0eb8f 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -195,7 +195,7 @@ SKIP: { # real socket
require_mods(qw(Cwd), my $nr = 46);
require PublicInbox::Spawn;
skip "Inline::C not installed/configured or IO::FDPass missing", $nr
- unless PublicInbox::Spawn->can('send_fd');
+ unless PublicInbox::Spawn->can('send_3fds');
local $ENV{XDG_RUNTIME_DIR} = "$home/xdg_run";
my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/sock";
diff --git a/t/spawn.t b/t/spawn.t
index e5cb09d9..891a3702 100644
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -8,20 +8,31 @@ use PublicInbox::Sigfd;
use Socket qw(AF_UNIX SOCK_STREAM);
SKIP: {
- my $recv_fd = PublicInbox::Spawn->can('recv_fd');
- my $send_fd = PublicInbox::Spawn->can('send_fd');
- skip 'Inline::C not enabled', 3 unless $send_fd && $recv_fd;
+ my $recv_3fds = PublicInbox::Spawn->can('recv_3fds');
+ my $send_3fds = PublicInbox::Spawn->can('send_3fds');
+ skip 'Inline::C not enabled', 3 unless $send_3fds && $recv_3fds;
my ($s1, $s2);
socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or BAIL_OUT $!;
pipe(my ($r, $w)) or BAIL_OUT $!;
- ok($send_fd->(fileno($s1), fileno($r)), 'pipe sent');
- my $rfd = $recv_fd->(fileno($s2));
- like($rfd, qr/\A\d+\z/, 'got FD');
- open(my $rfh, '<&=', $rfd) or BAIL_OUT $!;
- my @old = stat($r);
- my @new = stat($rfh);
- is("$old[0]\0$old[1]", "$new[0]\0$new[1]",
- 'device/inode matches on received FD');
+ my @orig = ($r, $w, $s2);
+ my @fd = map { fileno($_) } @orig;
+ ok($send_3fds->(fileno($s1), $fd[0], $fd[1], $fd[2]),
+ 'FDs sent');
+ my (@fds) = $recv_3fds->(fileno($s2));
+ is(scalar(@fds), 3, 'got 3 fds');
+ use Data::Dumper; diag Dumper(\@fds);
+ is(scalar(grep(/\A\d+\z/, @fds)), 3, 'all valid FDs');
+ my $i = 0;
+ my @cmp = map {
+ open my $new, $_, shift(@fds) or BAIL_OUT "open $! $i => $_";
+ ($new, shift(@orig), $i++);
+ } (qw(<&= >&= +<&=));
+ while (my ($new, $old, $fd) = splice(@cmp, 0, 3)) {
+ my @new = stat($new);
+ my @old = stat($old);
+ is("$old[0]\0$old[1]", "$new[0]\0$new[1]",
+ "device/inode matches on received FD:$fd");
+ }
}
{
^ permalink raw reply related [relevance 6%]
* [PATCH 0/2] fix race from stdout buffering in FD pass exit
@ 2021-01-03 11:24 7% ` Eric Wong
2021-01-03 11:24 6% ` [PATCH 1/2] send and receive all 3 FDs at once Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2021-01-03 11:24 UTC (permalink / raw)
To: meta
I implemented 1/2 thinking in hopes it would help, but it's
a nice syscall reduction anyways.
2/2 is the actual fix I've been struggling to find, and I could
only reproduce it on this FreeBSD VM I have access to. It seems
so obvious in retrospect :x
Eric Wong (2):
send and receive all 3 FDs at once
lei: fix output race in client/daemon mode
lib/PublicInbox/LEI.pm | 33 +++++++++++++++----------
lib/PublicInbox/Spawn.pm | 53 ++++++++++++++++++++++++++--------------
script/lei | 6 ++---
t/lei.t | 2 +-
t/spawn.t | 33 ++++++++++++++++---------
5 files changed, 80 insertions(+), 47 deletions(-)
^ permalink raw reply [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-01-03 9:48 [PATCH 0/3] lei-related test fixes Eric Wong
2021-01-03 11:24 7% ` [PATCH 0/2] fix race from stdout buffering in FD pass exit Eric Wong
2021-01-03 11:24 6% ` [PATCH 1/2] send and receive all 3 FDs at once Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
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).