FileAlreadyExistsException in hudson.Util.createSymlinkJava7

[Marco Miller]

Hi!

Some Jenkins instances of ours do log FileAlreadyExistsException-s from time to time:

java.nio.file.FileAlreadyExistsException: (path2onesJob)/lastSuccessful

at sun.nio.fs.UnixException.translateToIOException(UnixException.java:88)

(..) at hudson.Util.createSymlinkJava7(Util.java:1122)

(..)

-Above trace mapping to 1.509.2.

Code shows "proper" deletion of symlink prior to (re)creating it:

(..)

            filesC.getMethod("deleteIfExists", pathC).invoke(null, path);

            Object noAttrs = Array.newInstance(Class.forName("java.nio.file.attribute.FileAttribute"), 0);

            filesC.getMethod("createSymbolicLink", pathC, pathC, noAttrs.getClass()).invoke(null, path, target, noAttrs);

-Looking at that code and around,

I suspect some occasional race condition by lack of concurrency safeness:

1. thread-A deletes any existing lastSuccessful symlink (so far so good)

2. thread-B deletes it too but that is a no-op by now, given 1.

3. thread-A (re)creates the symlink (no problem yet)

4. thread-B recreates the symlink, but that throws our hereby exception, given 3. (problematic).

-Unless such initial deletion fails because of this:

"On some operating systems it may not be possible to remove a file when it is open and in use by this Java virtual machine or other programs." (*)

(*) http://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#deleteIfExists(java.nio.file.Path)

=> Q, please -assuming the above threading hypothesis and solving it to be hard & risky:

would you consider adding handling of that FileAlreadyExistsException, so createSymlinkJava7 logs it then return true (ok)?

-Thx for sharing your ideas,

Marco.

[Jesse Glick]

On Thu, Nov 14, 2013 at 3:13 PM, Marco Miller <miller...@me.com> wrote:
> 4. thread-B recreates the symlink, but that throws our hereby exception,
> given 3. (problematic).

Yeah, could happen.

> "On some operating systems it may not be possible to remove a file when it
> is open and in use by this Java virtual machine or other programs."

“Some” operating systems would be built in Redmond, and have mandatory
file locks. Do you run one of them? Anyway I doubt you can take out a
file lock on a symlink.

> would you consider adding handling of that FileAlreadyExistsException, so
> createSymlinkJava7 logs it then return true (ok)?

Logging a normal condition is pointless, and returning OK if we were
trying to _update_ a symlink to a different value would be a bug.
Probably the code should look for an existing symlink first, and try
to read it, and check that it matches what is already there. And if a
FAEE is thrown anyway, maybe retry a couple times to defeat race
conditions.

[Marco Miller]

Thx Jesse;

=> https://github.com/jenkinsci/jenkins/pull/1019