On 07/04/2018 03:35 PM, Paul Eggert wrote:
> Carlos O'Donell wrote:
>
>> If we really need another non-race-free API then gnulib can provide
>> that.
>
> We do really need it. All existing uses of renameat2-like
> functionality in GNU applications already use the non-race-free API.
> But as I see it, the consensus is to banish this functionality to
> Gnulib. Since it will longer matter to Gnulib whether Glibc
> introduces renameat2, I withdraw my objection to the patch.
>
> To help forestall likely bugs in code using Glibc renameat2, it would
> be helpful for the Glibc manual to document this issue, at least for
> RENAME_NOREPLACE which is the only reason I know anybody is calling
> renameat2 anyway. That is, if we're stuck with this API, at least we
> should warn users about its shortcomings. Perhaps the Glibc manual
> could point to Gnulib, as a suggestion for users who prefer the
> non-race-free functionality.
This is a good suggestion, and I think Florian should work on something
going into the manual to document the behaviour.
>> We are not "fighting" against GNU applications
> As an application writer it sure looks like a fight to me. Glibc is
> supplying an API that doesn't do what GNU apps need. So I plan to
> change the name of the Gnulib function to avoid namespace collisions,
> and as a result GnU apps via Gnulib will not use Glibc renameat2 at
> all. (Why should they bother? The Gnulib function already renames
> atomically using code that works on more platforms than Glibc does,
> and the Gnulib code is a tiny bit more efficient even when calling
> Glibc renameat2 would work.)
You position Gnulib's implementation as having no drawbacks, but this
is not true. The API has a race, and it is something which along with
other similar racy APIs has caused difficult to solve problems a the
distribution level.
I think it's a bad design choice to wrap renameat2 in a wrapper
that falls back to a racy solution. It can only cause us problems
because of the naming, in that renameat2 is not supposed to be racy.
However, I have no objection to a racy API with another name.
> I understand the desire to support an API that guarantees atomicity,
> and I'm not arguing against that. All I'm saying is that Glibc should
> also support use cases that merely prefer instead of requiring
> atomicity, as these are so common in practice that we still haven't
> run into a GNU app that actually *requires* atomicity.
coreutils *requires* it to solve the race, it says so in their NEWS:
~~~
'mv -n A B' no longer suffers from a race condition that can
overwrite a simultaneously-created B. This bug fix requires
^^^^^^^^
platform support for the renameat2 or renameatx_np syscalls, found
in recent Linux and macOS kernels. As a side effect, ‘mv -n A A’
now silently does nothing if A exists.
~~~
without it the fix regresses.
I don't object to a *distinct* API which implements the non-atomic variants.
I'm happy to review such new API additions, but that's not what is being
proposed in this thread by Florian.
In coreutils-8.30 lib/renameat2.c calls out at least 2 races which affect
correct operation. I would like *coretuils* itself to make the decision
in their sources, with full disclosure, to call the non-racy API first,
see if it works, and then if it doesn't work, attempt a racy API fallback.
This clearly documents the intended behaviour and is still easy for
application developers to use.
I don't think it's a good design choice to put both of those things together
into one API call. The semantics are sufficiently different that mistakes
will get made.
I want glibc to make a clear design decision here that these two things
should not be mixed in a single API, there is no strong reason to do so,
and in the past it has caused hard to find bugs.
--
Cheers,
Carlos.
