Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 397217

Summary: race condition affecting org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(...) when called concurrently from multiple threads
Product: [Technology] JGit Reporter: Quentin Burley <quentinburley>
Component: JGitAssignee: Project Inbox <jgit.core-inbox>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: quentinburley
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
[PATCH] call FileUtils.mkdir(dst.getParentFile(), true); i.e. with skipExisting=true. instead of FileUtils.mkdir(dst.getParentFile()); none

Description Quentin Burley CLA 2012-12-28 06:12:50 EST
When invoking org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(...) concurrently from multiple threads, the call to org.eclipse.jgit.util.FileUtils.mkdir(...) can fail in the case that more than one FileUtils.mkdir(...) have been invoked to create the same directory concurrently. 
This is more likely to occur with a new or not yet populated repository for which the directories .git\objects\?? have not been created yet.

An appropriate test case was used to confirm that the issue exists in at least the following jgit versions:
1.3.0.201202151440-r
2.0.0.201206130900-r
2.1.0.201209190230-r
2.2.0.201212191850-r
2.3.0-SNAPSHOT (build of git://git.eclipse.org/gitroot/jgit/jgit.git master latest commit 706f8eb9fc 20/12/2012)

The proposed solution is simple, instead of calling org.eclipse.jgit.util.FileUtils->"public static void mkdirs(final File d) throws IOException"
call instead org.eclipse.jgit.util.FileUtils->"public static void mkdir(final File d, boolean skipExisting) throws IOException" with skipExisting=true.

This can be achieved with a single modification to the file 
org.eclipse.jgit/src/org/eclipse/jgit/storage/file/ObjectDirectory.java line number:580

-    FileUtils.mkdir(dst.getParentFile());
+    FileUtils.mkdir(dst.getParentFile(), true);

An appropriate test case confirms that with this modification the issue is resolved.

Please find below stack traces obtained from a test case.

---

stack trace for versions:
2.0.0.201206130900-r
2.1.0.201209190230-r
2.2.0.201212191850-r
2.3.0-SNAPSHOT (build of git://git.eclipse.org/gitroot/jgit/jgit.git master latest commit 706f8eb9fc 20/12/2012)

Caused by: java.io.IOException: Creating directory C:\Java\NetBeansProjects\JgitObjectInserterMultithreadIssueTest\.\target\test\testRepo_290650716963937\.git\objects\00 failed
	at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:182)
	at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:160)
	at org.eclipse.jgit.storage.file.ObjectDirectory.insertUnpackedObject(ObjectDirectory.java:590)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insertOneObject(ObjectDirectoryInserter.java:113)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:91)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:101)
	at com.test.jgitobjectinsertermultithreadissuetest.GitObjectInserterJobThreadPool.doOperation(GitObjectInserterJobThreadPool.java:116)
	at com.test.jgitobjectinsertermultithreadissuetest.GitObjectInserterJobThreadPool.doOperation(GitObjectInserterJobThreadPool.java:24)
	at com.test.jgitobjectinsertermultithreadissuetest.GenericJobQueueThreadPool$ConsumerThread.run(GenericJobQueueThreadPool.java:93)

---

stack trace for version:
1.3.0.201202151440-r

Caused by: java.io.IOException: Creating directory C:\Java\NetBeansProjects\JgitObjectInserterMultithreadIssueTest\.\target\test\testRepo_291992513128641\.git\objects\63 failed
	at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:174)
	at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:152)
	at org.eclipse.jgit.storage.file.ObjectDirectory.insertUnpackedObject(ObjectDirectory.java:580)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insertOneObject(ObjectDirectoryInserter.java:113)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:91)
	at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:101)
	at com.test.jgitobjectinsertermultithreadissuetest.GitObjectInserterJobThreadPool.doOperation(GitObjectInserterJobThreadPool.java:116)
	at com.test.jgitobjectinsertermultithreadissuetest.GitObjectInserterJobThreadPool.doOperation(GitObjectInserterJobThreadPool.java:24)
	at com.test.jgitobjectinsertermultithreadissuetest.GenericJobQueueThreadPool$ConsumerThread.run(GenericJobQueueThreadPool.java:93)
Comment 1 Quentin Burley CLA 2012-12-28 07:02:13 EST
Created attachment 225082 [details]
[PATCH] call FileUtils.mkdir(dst.getParentFile(), true); i.e. with skipExisting=true.  instead of  FileUtils.mkdir(dst.getParentFile());

call FileUtils.mkdir(dst.getParentFile(), true);

i.e. with skipExisting=true.

instead of  FileUtils.mkdir(dst.getParentFile());

In order to resolve a race condition that occurs when different threads attempt to create the same directory concurrently.
Such a can condition occur when invoking org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(...) concurrently from multiple threads.
Comment 2 Quentin Burley CLA 2013-05-22 11:02:01 EDT
fixed by commit (author: rtyley):
https://github.com/eclipse/jgit/commit/5dcc8693d7d1ab81e3d6e6583a3918a3b6003bad

Fix concurrent creation of fan-out object directories
If multiple threads attempted to insert loose objects into the same new
fan-out directory, the creation of that directory was subject to a race
condition that could lead to an unnecessary IOException being thrown -
because an inserter could not 'create' a directory that had just been
generated by a different thread. All we require is that the directory
does indeed *exist*, so not being able to _create_ it is not actually a
fatal problem. Setting 'skipExisting' to 'true' on the call to mkdir()
fixes the issue.

I found this issue as a real world occurrence while working on The BFG
Repo Cleaner (https://github.com/rtyley/bfg-repo-cleaner), a tool which
concurrently performs a lot of object creation.

In order to demonstrate the problem here I've added a small test case
which reliably reproduces the issue on the few different hardware
systems I've tried. The error thrown when the race-condition arises is
this:

java.io.IOException: Creating directory /home/roberto/repo.git/objects/e6 failed
at org.eclipse.jgit.util.FileUtils.mkdir(FileUtils.java:182)
at org.eclipse.jgit.storage.file.ObjectDirectory.insertUnpackedObject(ObjectDirectory.java:590)
at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insertOneObject(ObjectDirectoryInserter.java:113)
at org.eclipse.jgit.storage.file.ObjectDirectoryInserter.insert(ObjectDirectoryInserter.java:91)
at org.eclipse.jgit.lib.ObjectInserter.insert(ObjectInserter.java:329)

Change-Id: I88eac49bc600c56ba9ad290e6133d8a7113125ab
 master   v3.0.0.201305080800-m7 
…
 v2.3.0.201302130906
Comment 3 Quentin Burley CLA 2013-05-22 11:02:58 EDT
fixed by commit (author: rtyley):
https://github.com/eclipse/jgit/commit/5dcc8693d7d1ab81e3d6e6583a3918a3b6003bad