[Bug libc/16285] New: thread-safe file locking (using lock file)

[Bug libc/16285] New: thread-safe file locking (using lock file)

[wolfgang at contre dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

            Bug ID: 16285
           Summary: thread-safe file locking (using lock file)
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: wolfgang at contre dot com
                CC: drepper.fsp at gmail dot com

As you know, lockf, flock and fcntl are not useful to guarantee the ownership
of a lock for an unexisting file. To my knowledge, the only way to guarantee
that a yet-to-be-created file is properly locked beforehand is by using the
O_CREAT flag with "open". Then, a dead lock problem occurs if this is performed
on the original file, as there would be a priori no way to know if an existing
file is actually being created or if its creator process died, thereby leading
to a dead lock. One possibility is thus to use a separate specific lockfile
containing the pid ot the tid of the calling thread, which could then be
checked for "staleness" by other threads by writing the active pid/tid to it
and checking it back. Now, the problem with this approach is that if the file
is checked by two simultaneous processes and that they both attempt to acquire
it, there is no way to ensure that one of them would not unlink the version
just created by the other process. This, in turn, causes a race condition over
the lock file, which takes us back to our original problem.

What I think is that a correct way to avoid deadlocking would be if the kernel
could somehow guarantee that a file could be unlinked from the filesystem as
soon as it is closed or if it's creator process terminates (which would also
imply the former). I would thus propose a "O_UNLINK" flag to "open" which would
allow that.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Rich Felker <bugdal at aerifal dot cx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugdal at aerifal dot cx

--- Comment #1 from Rich Felker <bugdal at aerifal dot cx> ---
Could you clarify the intended usage case? I'm not clear on why this
functionality is essential, and perhaps knowing what you need it for would help
in recommending an alternate solution.

In any case, new open flags are a kernel matter; glibc has nothing to do with
getting such a feature added, but would need to add the macro to its headers if
it were added.

BTW, the bits of the open flags int are almost exhausted, so asking for new
features there is really not appropriate unless they're extremely important.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[wolfgang at contre dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

--- Comment #2 from Wolfgang Sourdeau <wolfgang at contre dot com> ---
(In reply to Rich Felker from comment #1)
> Could you clarify the intended usage case? I'm not clear on why this
> functionality is essential, and perhaps knowing what you need it for would
> help in recommending an alternate solution.

Yes of course!

The use case is as follows... A multithreaded process that download files from
a remote connection and caches them on the local fs. A similar process may
already be running on the same machine or even a similar thread. Before the
download, we need to check whether a copy of the file already exists. If it
does, we open it, otherwise we create it and download its actual content.
Moreoever, there is to know that the cache location is intentionnally the same
for all threads and processes.

Initially, the problem was that a race condition would sometimes occur and the
same file would be downloaded simultaneously from 2 threads/processes or more,
overwriting one another's output. The fix I decided to use was a lock file
created with O_CREAT and O_EXCL, and where the active thread id (gettid) would
be put to enable other threads to identify the owner. Since this code is
actually part of a library, it may occur that the process will crash due to a
bug in another thread. Therefore I need to handle stale lock files by checking
for the tid's validity. If the tid is invalid, the lock file is unlinked and
recreated by the new thread.

Now, the situation that occurs here is another race condition, where between
the unlinking and the creation of the new file, another thread or process may
have actually read the old version and unlinked the new version, "thinking" it
was the old one. And that is the race condition I am trying to think about, in
the cleanest possible way. Arguable, I could retest whether I still own the
lock file after creating it and after a small delay, but I would not consider
this as a valid solution. Also, I have done some research on a few blogs and
q&a sites and never found a satisfactory answer to that problem.

> 
> In any case, new open flags are a kernel matter; glibc has nothing to do
> with getting such a feature added, but would need to add the macro to its
> headers if it were added.
> 
> BTW, the bits of the open flags int are almost exhausted, so asking for new
> features there is really not appropriate unless they're extremely important.

I understand but I figured that from my point of view it would be better to
start what the libc developers would think about this first before going
further. Simply because from a user program, the libs provides the relevant
interfaces.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

--- Comment #3 from Rich Felker <bugdal at aerifal dot cx> ---
Normally for this kind of thing, you write into a unique temporary file and
rename (which is atomic) to the actual name only when the file is complete.
This avoids all the issues you're concerned about, I believe.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[wolfgang at contre dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

--- Comment #4 from Wolfgang Sourdeau <wolfgang at contre dot com> ---
(In reply to Rich Felker from comment #3)
> Normally for this kind of thing, you write into a unique temporary file and
> rename (which is atomic) to the actual name only when the file is complete.
> This avoids all the issues you're concerned about, I believe.

(thank you for your quick responses!)

Actually the problem is still the same, for the reason that renaming can
overwrite the target file, which does not solve the race condition.
The other solution I thought about was to hard link a file, since this too
renders the file content atomically, but then it requires an unlink + link
phase which causes the race condition again.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[wolfgang at contre dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

--- Comment #5 from Wolfgang Sourdeau <wolfgang at contre dot com> ---
Also, I think this flag could be useful to perform an automatic cleanup
whenever a process dies, as a way to avoid stale temporary files, no matter
their actual purpose.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

--- Comment #6 from Rich Felker <bugdal at aerifal dot cx> ---
There are two types of renaming: the rename function, and link+unlink. Use the
latter if you want to avoid replacing an existing file. There may be other ways
to do this too.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[carmelo73 at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Carmelo Amoroso <carmelo73 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carmelo73 at gmail dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[neleai at seznam dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Ondrej Bilka <neleai at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |neleai at seznam dot cz
         Resolution|---                         |INVALID

--- Comment #7 from Ondrej Bilka <neleai at seznam dot cz> ---
I agree with Rich here, link with checking EEXIST suffices.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[wolfgang at contre dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Wolfgang Sourdeau <wolfgang at contre dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |---

--- Comment #8 from Wolfgang Sourdeau <wolfgang at contre dot com> ---
(In reply to Ondrej Bilka from comment #7)
> I agree with Rich here, link with checking EEXIST suffices.

No it does not. As mentionned above. EEXIST does not enable the safe removal of
stale lock files. The problem is the handling of stale lock files in a
multi-threaded program:
- either they exist indefinitely and the threads are locked until a manual
intervention is performed
- either they are replaced automatically with the risk of a race condition,
because using "link" requires "unlink" and 2 threads can execute "unlink"
simultaneously without knowing that they are removing a new valid lock file
from another active thread.

I can provide code that illustrate that if you wish but do a search on "linux
lock threads" and you will notice that I am not the only one having that issue.

An alternative method, which I have found meanwhile, is to use robust mutexes
with a mmapped file. That solution works perfectly well. I still think that my
solution would be elegant though, as it would also enable the automatic cleanup
of files following a program crash.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[neleai at seznam dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Ondrej Bilka <neleai at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |INVALID

--- Comment #9 from Ondrej Bilka <neleai at seznam dot cz> ---

> No it does not. As mentionned above. EEXIST does not enable the safe removal of 
> stale lock files. The problem is the handling of stale lock files in a multi-
> threaded program:
> - either they exist indefinitely and the threads are locked until a manual 
> intervention is performed
> - either they are replaced automatically with the risk of a race condition, 
> because using "link" requires "unlink" and 2 threads can execute "unlink" 
> simultaneously without knowing that they are removing a new valid lock file 
> from another active thread.

If file exists you could simply lock it. For safely unlinking first get lock,
then open file again and check if inodes agree and unlink file if they so.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16285] thread-safe file locking (using lock file)

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16285

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.