user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH] li2wrap: avoid double-close on Linux::Inotify2 <2.3
@ 2021-08-06  0:29  5% Eric Wong
  0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2021-08-06  0:29 UTC (permalink / raw)
  To: meta

LI2Wrap was not working as expected due to the missing bless
to override ->DESTROY.  This bug showed up in an message check in
t/lei-q-remote-import.t

Fixes: 7fc6e30aeab9925b ("lei: close inotify FD in forked child")
---
 lib/PublicInbox/LI2Wrap.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/LI2Wrap.pm b/lib/PublicInbox/LI2Wrap.pm
index 61cf4bee..204850a6 100644
--- a/lib/PublicInbox/LI2Wrap.pm
+++ b/lib/PublicInbox/LI2Wrap.pm
@@ -12,6 +12,7 @@ sub wrapclose {
 	my ($inot) = @_;
 	my $fd = $inot->fileno;
 	open my $fh, '<&=', $fd or die "open <&= $fd $!";
+	bless $inot, __PACKAGE__;
 }
 
 sub DESTROY {} # no-op

^ permalink raw reply related	[relevance 5%]

* Re: [RFC v2] lei: close inotify FD in forked child
  2021-07-29 10:01  6% ` [RFC v2] lei: close inotify FD in forked child Eric Wong
@ 2021-08-04 10:40  7%   ` Eric Wong
  0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2021-08-04 10:40 UTC (permalink / raw)
  To: meta

Pushed with present tense commit message now that L::I2 2.3+ is out:

https://public-inbox.org/meta/7fc6e30aeab9925bece4bb00f88bb91af5646aa2/s/

^ permalink raw reply	[relevance 7%]

* [RFC v2] lei: close inotify FD in forked child
  @ 2021-07-29 10:01  6% ` Eric Wong
  2021-08-04 10:40  7%   ` Eric Wong
  0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2021-07-29 10:01 UTC (permalink / raw)
  To: meta

It looks like Linux::Inotify2 2.3+ will include an ->fh method
to give us the ability to safely close an FD without hitting
EBADF (and automatically use FD_CLOEXEC).

We'll still need a new wrapper class (LI2Wrap) to handle it for
users of old versions, though.

http://lists.schmorp.de/pipermail/perl/2021q3/thread.html
---
 MANIFEST                   |  1 +
 lib/PublicInbox/DirIdle.pm | 11 +++++++++++
 lib/PublicInbox/LEI.pm     |  2 +-
 lib/PublicInbox/LI2Wrap.pm | 20 ++++++++++++++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 lib/PublicInbox/LI2Wrap.pm

diff --git a/MANIFEST b/MANIFEST
index a3913501..fb9f16bf 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -197,6 +197,7 @@ lib/PublicInbox/InputPipe.pm
 lib/PublicInbox/Isearch.pm
 lib/PublicInbox/KQNotify.pm
 lib/PublicInbox/LEI.pm
+lib/PublicInbox/LI2Wrap.pm
 lib/PublicInbox/LeiALE.pm
 lib/PublicInbox/LeiAddWatch.pm
 lib/PublicInbox/LeiAuth.pm
diff --git a/lib/PublicInbox/DirIdle.pm b/lib/PublicInbox/DirIdle.pm
index 65896f95..d572c274 100644
--- a/lib/PublicInbox/DirIdle.pm
+++ b/lib/PublicInbox/DirIdle.pm
@@ -84,4 +84,15 @@ sub event_step {
 	warn "$self->{inot}->read err: $@\n" if $@;
 }
 
+sub force_close {
+	my ($self) = @_;
+	my $inot = delete $self->{inot} // return;
+	if ($inot->can('fh')) { # Linux::Inotify2 2.3+
+		close($inot->fh) or warn "CLOSE ERROR: $!";
+	} elsif ($inot->isa('Linux::Inotify2')) {
+		require PublicInbox::LI2Wrap;
+		PublicInbox::LI2Wrap::wrapclose($inot);
+	}
+}
+
 1;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index d9fd40fd..e6f763e1 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -556,7 +556,7 @@ sub _lei_atfork_child {
 	}
 	close $listener if $listener;
 	undef $listener;
-	undef $dir_idle;
+	$dir_idle->force_close if $dir_idle;
 	%PATH2CFG = ();
 	$MDIR2CFGPATH = {};
 	eval 'no warnings; undef $PublicInbox::LeiNoteEvent::to_flush';
diff --git a/lib/PublicInbox/LI2Wrap.pm b/lib/PublicInbox/LI2Wrap.pm
new file mode 100644
index 00000000..b0f4f8b8
--- /dev/null
+++ b/lib/PublicInbox/LI2Wrap.pm
@@ -0,0 +1,20 @@
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# Wrapper for Linux::Inotify2 < 2.3 which lacked ->fh and auto-close
+# Remove this when supported LTS/enterprise distros are all
+# Linux::Inotify2 >= 2.3
+package PublicInbox::LI2Wrap;
+use v5.10.1;
+our @ISA = qw(Linux::Inotify2);
+
+sub wrapclose {
+	my ($inot) = @_;
+	my $fd = $inot->fileno;
+	open my $fh, '<&=', $fd or die "open <&= $fd $!";
+
+}
+
+sub DESTROY {} # no-op
+
+1

^ permalink raw reply related	[relevance 6%]

Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-07-28 12:34     [RFC] lei: address lifetime problems from Linux::Inotify2 Eric Wong
2021-07-29 10:01  6% ` [RFC v2] lei: close inotify FD in forked child Eric Wong
2021-08-04 10:40  7%   ` Eric Wong
2021-08-06  0:29  5% [PATCH] li2wrap: avoid double-close on Linux::Inotify2 <2.3 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).