* [PATCH 1/2] tmpfile: give temporary files meaningful names
2019-09-12 8:34 [PATCH 0/2] tmpfile: new class to manage temporary files Eric Wong
@ 2019-09-12 8:34 ` Eric Wong
2019-09-12 8:34 ` [PATCH 2/2] tmpfile: support O_APPEND and use it in DS::tmpio Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-09-12 8:34 UTC (permalink / raw)
To: meta
Although we always unlink temporary files, give them a
meaningful name so that we can we can still make sense
of the pre-unlink name when using lsof(8) or similar
tools on Linux.
---
MANIFEST | 1 +
lib/PublicInbox/Git.pm | 4 +++-
lib/PublicInbox/GitHTTPBackend.pm | 4 +++-
lib/PublicInbox/HTTP.pm | 10 ++++----
lib/PublicInbox/SolverGit.pm | 3 ++-
lib/PublicInbox/Tmpfile.pm | 38 +++++++++++++++++++++++++++++++
lib/PublicInbox/ViewVCS.pm | 3 ++-
7 files changed, 55 insertions(+), 8 deletions(-)
create mode 100644 lib/PublicInbox/Tmpfile.pm
diff --git a/MANIFEST b/MANIFEST
index 24280351..777367d0 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -135,6 +135,7 @@ lib/PublicInbox/Spawn.pm
lib/PublicInbox/SpawnPP.pm
lib/PublicInbox/Syscall.pm
lib/PublicInbox/TLS.pm
+lib/PublicInbox/Tmpfile.pm
lib/PublicInbox/Unsubscribe.pm
lib/PublicInbox/UserContent.pm
lib/PublicInbox/V2Writable.pm
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index d048051e..ff3838b3 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -12,6 +12,7 @@ use warnings;
use POSIX qw(dup2);
require IO::Handle;
use PublicInbox::Spawn qw(spawn popen_rd);
+use PublicInbox::Tmpfile;
use base qw(Exporter);
our @EXPORT_OK = qw(git_unquote git_quote);
@@ -110,7 +111,8 @@ sub _bidi_pipe {
qw(-c core.abbrev=40 cat-file), $batch);
my $redir = { 0 => fileno($out_r), 1 => fileno($in_w) };
if ($err) {
- open(my $fh, '+>', undef) or fail($self, "open.err failed: $!");
+ my $id = "git.$self->{git_dir}$batch.err";
+ my $fh = tmpfile($id) or fail($self, "tmpfile($id): $!");
$self->{$err} = $fh;
$redir->{2} = fileno($fh);
}
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index c8878145..a8337035 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -12,6 +12,7 @@ use HTTP::Date qw(time2str);
use HTTP::Status qw(status_message);
use Plack::Util;
use PublicInbox::Qspawn;
+use PublicInbox::Tmpfile;
# 32 is same as the git-daemon connection limit
my $default_limiter = PublicInbox::Qspawn::Limiter->new(32);
@@ -218,7 +219,8 @@ sub input_prepare {
if (defined $fd && $fd >= 0) {
return { 0 => $fd };
}
- open(my $in, '+>', undef);
+ my $id = "git-http.input.$env->{REMOTE_HOST}:$env->{REMOTE_PORT}";
+ my $in = tmpfile($id);
unless (defined $in) {
err($env, "could not open temporary file: $!");
return;
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 19b57d59..b43ef870 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -21,6 +21,7 @@ use IO::Handle;
require PublicInbox::EvCleanup;
use PublicInbox::DS qw(msg_more);
use PublicInbox::Syscall qw(EPOLLIN EPOLLONESHOT);
+use PublicInbox::Tmpfile;
use constant {
CHUNK_START => -1, # [a-f0-9]+\r\n
CHUNK_END => -2, # \r\n
@@ -325,8 +326,9 @@ sub response_write {
}
sub input_tmpfile ($) {
- open($_[0], '+>', undef);
- $_[0]->autoflush(1);
+ my $input = tmpfile('http.input', $_[0]->{sock}) or return;
+ $input->autoflush(1);
+ $input;
}
sub input_prepare {
@@ -338,10 +340,10 @@ sub input_prepare {
quit($self, 413);
return;
}
- input_tmpfile($input);
+ $input = input_tmpfile($self);
} elsif (env_chunked($env)) {
$len = CHUNK_START;
- input_tmpfile($input);
+ $input = input_tmpfile($self);
} else {
$input = $null_io;
}
diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm
index 49f94895..8878961e 100644
--- a/lib/PublicInbox/SolverGit.pm
+++ b/lib/PublicInbox/SolverGit.pm
@@ -15,6 +15,7 @@ use Fcntl qw(SEEK_SET);
use PublicInbox::Git qw(git_unquote git_quote);
use PublicInbox::MsgIter qw(msg_iter msg_part_text);
use PublicInbox::Qspawn;
+use PublicInbox::Tmpfile;
use URI::Escape qw(uri_escape_utf8);
# POSIX requires _POSIX_ARG_MAX >= 4096, and xargs is required to
@@ -235,7 +236,7 @@ sub prepare_index ($) {
my $path_a = $di->{path_a} or die "BUG: path_a missing for $oid_full";
my $mode_a = $di->{mode_a} || extract_old_mode($di);
- open my $in, '+>', undef or die "open: $!";
+ my $in = tmpfile("update-index.$oid_full") or die "tmpfile: $!";
print $in "$mode_a $oid_full\t$path_a\0" or die "print: $!";
$in->flush or die "flush: $!";
sysseek($in, 0, 0) or die "seek: $!";
diff --git a/lib/PublicInbox/Tmpfile.pm b/lib/PublicInbox/Tmpfile.pm
new file mode 100644
index 00000000..7fda100e
--- /dev/null
+++ b/lib/PublicInbox/Tmpfile.pm
@@ -0,0 +1,38 @@
+# Copyright (C) 2019 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::Tmpfile;
+use strict;
+use warnings;
+use base qw(Exporter);
+our @EXPORT = qw(tmpfile);
+use Fcntl qw(:DEFAULT);
+use Errno qw(EEXIST);
+require File::Spec;
+
+# use tmpfile instead of open(..., '+>', undef) so we can get an
+# unlinked filename which makes sense when viewed with lsof
+# (at least on Linux)
+# TODO: O_APPEND support (this is the reason I'm not using File::Temp)
+# And if we ever stop caring to have debuggable filenames, O_TMPFILE :)
+sub tmpfile ($;$) {
+ my ($id, $sock) = @_;
+ if (defined $sock) {
+ # add the socket inode number so we can figure out which
+ # socket it belongs to
+ my @st = stat($sock);
+ $id .= '-ino:'.$st[1];
+ }
+ $id =~ tr!/!^!;
+
+ my $fl = O_RDWR | O_CREAT | O_EXCL;
+ do {
+ my $fn = File::Spec->tmpdir . "/$id-".time.'-'.rand;
+ if (sysopen(my $fh, $fn, $fl, 0600)) { # likely
+ unlink($fn) or die "unlink($fn): $!"; # FS broken
+ return $fh; # success
+ }
+ } while ($! == EEXIST);
+ undef # EMFILE/ENFILE/ENOSPC/ENOMEM
+}
+
+1;
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 60a62e57..369afe93 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -20,6 +20,7 @@ use bytes (); # only for bytes::length
use PublicInbox::SolverGit;
use PublicInbox::WwwStream;
use PublicInbox::Linkify;
+use PublicInbox::Tmpfile;
use PublicInbox::Hval qw(ascii_html to_filename);
my $hl = eval {
require PublicInbox::HlMod;
@@ -185,7 +186,7 @@ sub show ($$;$) {
$hints->{$to} = $v;
}
- open my $log, '+>', undef or die "open: $!";
+ my $log = tmpfile("solve.$oid_b");
my $solver = PublicInbox::SolverGit->new($ctx->{-inbox}, sub {
solve_result($ctx, $_[0], $log, $hints, $fn);
});
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] tmpfile: support O_APPEND and use it in DS::tmpio
2019-09-12 8:34 [PATCH 0/2] tmpfile: new class to manage temporary files Eric Wong
2019-09-12 8:34 ` [PATCH 1/2] tmpfile: give temporary files meaningful names Eric Wong
@ 2019-09-12 8:34 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-09-12 8:34 UTC (permalink / raw)
To: meta
Might as well share some code for temporary file creation
---
lib/PublicInbox/DS.pm | 14 ++++----------
lib/PublicInbox/Tmpfile.pm | 8 ++++----
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 1e51dc41..30a9641a 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -26,6 +26,7 @@ use warnings;
use 5.010_001;
use PublicInbox::Syscall qw(:epoll);
+use PublicInbox::Tmpfile;
use fields ('sock', # underlying socket
'rbuf', # scalarref, usually undef
@@ -33,7 +34,7 @@ use fields ('sock', # underlying socket
'wbuf_off', # offset into first element of wbuf to start writing at
);
-use Errno qw(EAGAIN EINVAL EEXIST);
+use Errno qw(EAGAIN EINVAL);
use Carp qw(croak confess carp);
require File::Spec;
@@ -488,15 +489,8 @@ sub drop {
# PerlIO::mmap or PerlIO::scalar if needed
sub tmpio ($$$) {
my ($self, $bref, $off) = @_;
- my $fh; # open(my $fh, '+>>', undef) doesn't set O_APPEND
- do {
- my $fn = File::Spec->tmpdir . '/wbuf-' . rand;
- if (sysopen($fh, $fn, O_RDWR|O_CREAT|O_EXCL|O_APPEND, 0600)) { # likely
- unlink($fn) or return drop($self, "unlink($fn) $!");
- } elsif ($! != EEXIST) { # EMFILE/ENFILE/ENOSPC/ENOMEM
- return drop($self, "open: $!");
- }
- } until (defined $fh);
+ my $fh = tmpfile('wbuf', $self->{sock}, 1) or
+ return drop($self, "tmpfile $!");
$fh->autoflush(1);
my $len = bytes::length($$bref) - $off;
$fh->write($$bref, $len, $off) or return drop($self, "write ($len): $!");
diff --git a/lib/PublicInbox/Tmpfile.pm b/lib/PublicInbox/Tmpfile.pm
index 7fda100e..28e87f88 100644
--- a/lib/PublicInbox/Tmpfile.pm
+++ b/lib/PublicInbox/Tmpfile.pm
@@ -12,10 +12,9 @@ require File::Spec;
# use tmpfile instead of open(..., '+>', undef) so we can get an
# unlinked filename which makes sense when viewed with lsof
# (at least on Linux)
-# TODO: O_APPEND support (this is the reason I'm not using File::Temp)
# And if we ever stop caring to have debuggable filenames, O_TMPFILE :)
-sub tmpfile ($;$) {
- my ($id, $sock) = @_;
+sub tmpfile ($;$$) {
+ my ($id, $sock, $append) = @_;
if (defined $sock) {
# add the socket inode number so we can figure out which
# socket it belongs to
@@ -25,10 +24,11 @@ sub tmpfile ($;$) {
$id =~ tr!/!^!;
my $fl = O_RDWR | O_CREAT | O_EXCL;
+ $fl |= O_APPEND if $append;
do {
my $fn = File::Spec->tmpdir . "/$id-".time.'-'.rand;
if (sysopen(my $fh, $fn, $fl, 0600)) { # likely
- unlink($fn) or die "unlink($fn): $!"; # FS broken
+ unlink($fn) or warn "unlink($fn): $!"; # FS broken
return $fh; # success
}
} while ($! == EEXIST);
^ permalink raw reply related [flat|nested] 3+ messages in thread