Hi,

Caleb Ristvedt <caleb.ristvedt@cune.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> Two questions:
>>
>>   1. Can another process come and fiddle with TO-REGISTER while we’re
>>      still in ‘reset-timestamps’?  Or can GC happen while we’re in
>>      ‘reset-timestamps’ and delete TO-REGISTER and remove it from the
>>      database?
>
>>
>> I think none of these scenarios can happen, as long as we’ve taken the
>> .lock file for TO-REGISTER before, like ‘finalize-store-file’ does.
>
> The lock file has no bearing on liveness of the path it locks, though
> the liveness of the path it locks *does* count as liveness for the lock
> file and for the .chroot file; see tryToDelete() in nix/libstore/gc.cc.
>
> (Well, semi-liveness; .lock and .chroot files won't show up in a list of
> live paths, but they will still be protected from deletion if their
> associated store item is a temp root)
>
> So general "fiddling" can't happen, but GC can. The responsibility for
> both locking and registering as temporary roots falls on the caller,
> currently, as I believe it should. We may want to document this
> responsibility in the docstring for register-items, though.

Yes, it would be good to add a sentence to document it.

> So currently finalize-store-file is safe from clobbering by another
> process attempting to finalize to the same path, but not safe from
> garbage collection between when the temporary store file is renamed and
> when it is registered. It needs an add-temp-root prior to renaming.

Ah, so we’d need to do that before applying the patch that reduces the
scope of the transaction, right?

>>   2. After the transaction, TO-REGISTER is considered valid.  But are
>>      the effects of the on-going deduplication observable, due to
>>      non-atomicity of some operation?
>
> Subdirectories of store items need to be made writable prior to renaming
> the temp link, so there will necessarily be a window of time during
> which various subdirectories will appear writable. I don't think this
> should cause problems; we already assume that the .lock file is held, so
> we should be the only process attempting to deduplicate it. On a related
> note, we should probably use dynamic-wind for setting and restoring the
> permissions in replace-with-link.

Yes.  Here’s a patch for ‘dynamic-wind’: