From: "Pádraig Brady" <P@draigBrady.com>
To: "Paul Eggert" <eggert@cs.ucla.edu>, "Ludovic Courtès" <ludo@gnu.org>
Cc: 21460-done@debbugs.gnu.org, bug-guix@gnu.org
Subject: bug#21460: Race condition in tests/tail-2/assert.sh
Date: Fri, 2 Oct 2015 16:50:22 +0100 [thread overview]
Message-ID: <560EA7BE.8010305@draigBrady.com> (raw)
In-Reply-To: <55F38C54.8070306@draigBrady.com>
[-- Attachment #1: Type: text/plain, Size: 776 bytes --]
On 12/09/15 03:22, Pádraig Brady wrote:
> On 12/09/15 02:09, Paul Eggert wrote:
>> Pádraig Brady wrote:
>>> + else if (new_stats.st_nlink == 0) /* XXX: what about multi-linked files. */
>>
>> That comment was my thought exactly. It appears to be a kernel bug that really
>> needs to get fixed in the kernel; there just doesn't seem to be a reliable
>> workaround in user space.
>
> I double checked with kernel guys, and Al Viro
> essentially said the inode and directory operations are independent.
> https://lkml.org/lkml/2015/9/11/790
>
> So we probably need to handle the IN_DELETE event
> on the directory to cater for this race, as done in:
> https://lists.gnu.org/archive/html/coreutils/2015-07/msg00015.html
I'll apply the attached later.
thanks,
Pádraig
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tail-unlink-notification-race.patch --]
[-- Type: text/x-patch; name="tail-unlink-notification-race.patch", Size: 3421 bytes --]
From 07ca8a227e04f9fb7bb0b21968056a562b8c2f83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Thu, 2 Jul 2015 08:41:25 +0100
Subject: [PATCH] tail: handle kernel dentry unlink race
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Avoid the intermittent loss of "... has become inaccessible" messages.
That would cause tests/tail-2/assert.sh to fail sometimes,
mainly on uniprocessor systems.
* src/tail.c (tail_forever_inotify): Also monitor IN_DELETE
events on the directory, to avoid a dentry unlink()..open() race,
where the open() on the deleted file was seen to succeed after an,
unlink() and a subsequent IN_ATTRIB, was sent to tail. Note an
IN_ATTRIB is sent on the monitored file to indicate the change in
number of links, and we can't just use a decrease in the number of
links to determine the file being unlinked, due to the possibility
of the file having multiple links.
Reported by Assaf Gordon and Ludovic Courtès.
Fixes http://bugs.gnu.org/21460
---
src/tail.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/tail.c b/src/tail.c
index f916d74..7a34b0b 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1429,8 +1429,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
/* It's fine to add the same directory more than once.
In that case the same watch descriptor is returned. */
f[i].parent_wd = inotify_add_watch (wd, dirlen ? f[i].name : ".",
- (IN_CREATE | IN_MOVED_TO
- | IN_ATTRIB));
+ (IN_CREATE | IN_DELETE
+ | IN_MOVED_TO | IN_ATTRIB));
f[i].name[dirlen] = prev;
@@ -1619,9 +1619,16 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
fspec = &(f[j]);
- /* Adding the same inode again will look up any existing wd. */
- int new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask);
- if (new_wd < 0)
+ int new_wd = -1;
+ bool deleting = !! (ev->mask & IN_DELETE);
+
+ if (! deleting)
+ {
+ /* Adding the same inode again will look up any existing wd. */
+ new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask);
+ }
+
+ if (! deleting && new_wd < 0)
{
if (errno == ENOSPC || errno == ENOMEM)
{
@@ -1641,7 +1648,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
/* This will be false if only attributes of file change. */
bool new_watch = fspec->wd < 0 || new_wd != fspec->wd;
- if (new_watch)
+ if (! deleting && new_watch)
{
if (0 <= fspec->wd)
{
@@ -1683,7 +1690,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files,
if (! fspec)
continue;
- if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
+ if (ev->mask & (IN_ATTRIB | IN_DELETE | IN_DELETE_SELF | IN_MOVE_SELF))
{
/* Note for IN_MOVE_SELF (the file we're watching has
been clobbered via a rename) we leave the watch
--
2.5.0
next prev parent reply other threads:[~2015-10-02 15:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-11 16:23 bug#21459: Race condition in tests/tail-2/assert.sh Ludovic Courtès
2015-09-11 17:18 ` bug#21460: " Paul Eggert
2015-09-11 20:55 ` Ludovic Courtès
2015-09-11 22:49 ` Pádraig Brady
2015-09-11 23:48 ` Pádraig Brady
2015-09-12 1:09 ` Paul Eggert
2015-09-12 2:22 ` Pádraig Brady
2015-10-02 15:50 ` Pádraig Brady [this message]
2015-09-11 20:34 ` bug#21459: " Pádraig Brady
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=560EA7BE.8010305@draigBrady.com \
--to=p@draigbrady.com \
--cc=21460-done@debbugs.gnu.org \
--cc=bug-guix@gnu.org \
--cc=eggert@cs.ucla.edu \
--cc=ludo@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.