Eric Wong <e...@yhbt.net> wrote: > --- a/lib/PublicInbox/Inbox.pm Naming is (still) hard.
> +# $obj must respond to >inbox_changed, which takes Inbox ($self) as an arg ^^^^^^^^^^^^^ That should be ->on_inbox_unlock > +sub subscribe_unlock { > + my ($self, $ident, $obj) = @_; > + $self->{over_subs}->{$ident} = $obj; > +} > + > +sub unsubscribe_unlock { > + my ($self, $ident) = @_; > + delete $self->{over_subs}->{$ident}; > +} {over_subs} might be a bad name, here. I originally had this watching over.sqlite3, but started watching the lock, instead. I figure SQLite3 could (now or in the future) write via mmap without waking up inotify. And there may be users who want to serve v1 w/o SQLite... > + > +# called by inotify > +sub on_unlock { > + my ($self) = @_; > + my $subs = $self->{over_subs} or return; > + for (values %$subs) { > + eval { $_->on_inbox_unlock($self) }; > + } Yes, ->on_inbox_unlock is the correct method right now, not ->inbox_changed > diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm > index 032841ed..5a55c9d3 100644 > --- a/lib/PublicInbox/Lock.pm > +++ b/lib/PublicInbox/Lock.pm > @@ -14,7 +14,7 @@ sub lock_acquire { > my ($self) = @_; > croak 'already locked' if $self->{lockfh}; > my $lock_path = $self->{lock_path} or return; > - sysopen(my $lockfh, $lock_path, O_WRONLY|O_CREAT) or > + sysopen(my $lockfh, $lock_path, O_TRUNC|O_WRONLY|O_CREAT) or I don't think O_TRUNC is necessary, here. O_TRUNC here bothers me, since it causes useless I/O traffic and SSD write amplification on systems, especially where inotify or kqueue are available. > @@ -24,6 +24,11 @@ sub lock_release { > my ($self) = @_; > return unless $self->{lock_path}; > my $lockfh = delete $self->{lockfh} or croak 'not locked'; > + > + # NetBSD 8.1 and OpenBSD 6.5 (and maybe other versions/*BSDs) lack > + # NOTE_CLOSE_WRITE from FreeBSD 11+, so trigger NOTE_WRITE, instead > + syswrite($lockfh, '.') if $^O ne 'linux'; Yeah, relying on NOTE_WRITE there is kinda gross, but NOTE_CLOSE_WRITE isn't available on all *BSDs. > + > +done_testing; > + > +package InboxIdleTestObj; > +use strict; > + > +sub new { bless {}, shift } > + > +sub on_inbox_unlock { > + my ($self, $ibx) = @_; > + push @{$self->{called}}, $ibx; > +} Yup, we use ->on_inbox_unlock, not ->inbox_changed -- unsubscribe: one-click, see List-Unsubscribe header archive: https://public-inbox.org/meta/