Re: [egit-dev] locking mechanism

[Matthias Sohn <matthias.sohn@xxxxxxxxx>]

On Wed, Dec 18, 2013 at 10:52 AM, Laurent Goubet <laurent.goubet@xxxxxxx> wrote:

Hi all,

I am not familiar with the locking mechanism that's been used in JGit, I think it is too limited, but I don't really know if it would be possible (and how) to make it a little friendlier. Consider the very simple test I attach to this mail (I had it as org.eclipse.egit.ui.test\src\org\eclipse\egit\ui\DeleteTest.java ... but its location is somewhat irrelevant).

The gist of it is : if I lock a repository's dir cache, I can no longer manipulate the workspace files supervised by it, even from sub-calls "under" my lock, but that's inconsistent :

DirCache dirCache = repo.lockDirCache();

IFile file1 = iProject.getFile(new Path("file1"));
file1.create(new ByteArrayInputStream(new byte[0]), false, new NullProgressMonitor()); // <- I can "create" a file just fine

file1.delete(false, new NullProgressMonitor()); // <- but tring to delete it fails miserably

dirCache.unlock();

I understand that what happens here is that EGit fails because of a workspace hook to automatically add "deleted" files to the index, but that hook cannot lock the index since I've already taken that lock beforehand.

If you don't add the newly created file to the git index explicitly jgit will not care for this untracked file.

So if deleting this untracked file fails (on Windows) probably some other process has an open handle.

Locking the DirCache is using a filesystem lock (index.lock in .git directory) in order to allow concurrent

use of c git and jgit on the same repository instance. It only locks the index file itself, see DirCache.lock().

My questions would be :

- Why are we only "automatically adding to the index" when the user deletes files, but not when he adds some? I think that if we do one, we should do the other. Not to mention that here, the hook fails to automatically stage the deletion of a file which creation had yet to be staged (i.e. a no-op).

this isn't done for untracked files. I think we could change this behavior for tracked files but I don't know

why egit does this.

 

- Since I call "delete" in a place where I _do_ hold the DirCache's lock, should this really fail? There should be a way for the delete hook to get a hold of the lock its thread holds, but how could I go about making this lock thread local?

This is a very simple test case, but it actually mimics a behavior I have seen from within a rebase operation (I am currently looking at how to make the rebase/cherry-pick/merge/... aware of logical models). Basically, this locking mechanism fails when there are multiple "worlds" in effect : when I make EGit interact with Team or other Eclipse plugins, there is a chance that that "other plugin" will try and make changes to the workspace. Eclipse handles that gracefully : I am within a WorkspaceOperation, I hold the lock for the containing IProject, I don't need to re-lock the file I need to change. JGit does not fare as good : I hold the index lock, yet I still try to re-lock it from a callee.

We should be able to share locks with callees, and I think the "current Thread" is a good context for this sharing... Yet I wonder if there were other reasons for this in JGit other than mimicking the cgit behavior?

the basic lock mechanism must be interoperable with c git since we want to be able to use both implementations

concurrently on the same repository instance.

--

Matthias