Re: [egit-dev] Fwd: locking mechanism

[Laurent Goubet <laurent.goubet@xxxxxxx>]

I think that was my mistake actually :).

Either way, comments inline below.

On 19/12/2013 09:41, Matthias Sohn wrote:

missed to send this to the list

---------- Forwarded message ----------
From: Matthias Sohn <matthias.sohn@xxxxxxxxx>
Date: Thu, Dec 19, 2013 at 9:39 AM
Subject: Re: [egit-dev] locking mechanism
To: Laurent Goubet <laurent.goubet@xxxxxxx>

On Thu, Dec 19, 2013 at 9:20 AM, Laurent Goubet <laurent.goubet@xxxxxxx> wrote:

On 18/12/2013 17:07, Matthias Sohn wrote:

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().

This simple test _does_ fail on windows, and I am pretty sure it will fail on linux too. Nothing else is in the mix other than EGit, JGit and Eclipse. On the first line, I lock the dir cache explicitely, I then create a new file without adding it to the index, and finally I try to delete that new, untracked file. That fails in IOException because of EGit's deletion hook that tries to lock the index.

This example code didn't mention that you try to run it inside EGit so if you run the above code 

standalone or as a jgit unit test it should succeed.

If you run this in EGit the file deletion won't fail because of a locked DirCache but you won't be able to

lock the DirCache a second time hence you never reach the statement trying to delete the file.

No, I do reach the deletion code (file1.delete(...) in the above example). It is below this delete() call that the hook fails.

Looks like either we need to get rid of the hook (not sure what's the reason for auto-staging deletions, probably similar

problems will surface for moves) or we need a way to share the DirCache instance (are both methods

trying to obtain the DirCache lock running in the same thread ?).

Yes. You can copy the unit test I attached to my first mail in the egit.tests plugin (or I could push a review for this if it makes things simpler).

I think In order to stay interoperable with c git we can't give up on using the existing file locking mechanism.

I also think it is important to keep the file lock, I just think that it should be possible to share the locked DirCache within the thread if at all possible.

Laurent

begin:vcard
fn:Laurent Goubet
n:Goubet;Laurent
org:<a href="http://www.obeo.fr";>Obeo</a>
email;internet:laurent.goubet@xxxxxxx
url:http://www.obeo.fr
version:2.1
end:vcard