lockfile icon indicating copy to clipboard operation
lockfile copied to clipboard

Documentation of algorithm constraints and fail on their violation

Open nightlyone opened this issue 7 years ago • 2 comments

Without the specific atomicity guarantees of setting a hard link and rename a file in a directory the whole locking beaks. I could be more friendly to the users by documenting these constraints better and being more specific about which errors I ignore.

Also fail properly on unexpected errors when creating the hardlink.

nightlyone avatar Aug 02 '17 06:08 nightlyone

Is the following a correct explanation of the semantics?

  1. Create a file at path dir+"."+name called tmplock. Fail if unable to create. (potential race condition here, I suggest it be dir+"."+name+random_stuff_or_pid_or_something)
  2. Write the current pid to tmplock, fail if unable to write.
  3. Hard link tmplock to dir+name. Ignore failure to link.
  4. Stat tmplock by name (should still be dir+"."+name). It should still exist, error if not.
  5. Stat the real lock dir+name. Should exist. Error if doesn't.
  6. Make sure that both the tmplock and the new lock are the same file.
  7. Get the owner of the lock. etc

matjam avatar Aug 02 '17 17:08 matjam

@matjam Thanks for taking the time to analyse the code deeply.

Is the following a correct explanation of the semantics?

  1. Create a file at path dir+"."+name called tmplock. Fail if unable to create. (potential race condition here, I suggest it be dir+"."+name+random_stuff_or_pid_or_something)

ioutil.TempFile takes care of the "random_stuff_or_pid_or_something" for me. So consider it done :slightly_smiling_face:

  1. Write the current pid to tmplock, fail if unable to write.

Yes.

  1. Hard link tmplock to dir+name. Ignore failure to link.

Your observation of the existing code is correct.

Intention here was to only ignore the failure to create a link due to the destination already existing. Ignoring every error here was the bug that made analysis of https://github.com/golang/dep/issues/913 harder than necessary. This imprecise error handling is fixed by https://github.com/nightlyone/lockfile/pull/21 .

  1. Stat tmplock by name (should still be dir+"."+name). It should still exist, error if not.
  2. Stat the real lock dir+name. Should exist. Error if doesn't.
  3. Make sure that both the tmplock and the new lock are the same file.
  4. Get the owner of the lock. etc

correct.

nightlyone avatar Aug 02 '17 22:08 nightlyone