The one culprit is the use of
sub-modules displayed in the git repositories view.
http://git.eclipse.org/c/egit/egit.git/tree/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java#n430
We use a SubmoduleWalk to create Repository instances. JGit will
fully parse the repository in question (which includes opening
channels towards any "pack" file), but that repository will never
be cached in egit.core.Activator.repositoryCache.... which means
that we'll never call "close()" on it, never releasing the locks
on these pack files. WindowCache _does_ use WeakReferences for
these locks... but these particular references cannot be cleared
automatically by the GC: when a Repository is displayed in the Git
repositories view, they are wrapped within a "RepositoryNode",
these RepositoryNodes are cached themselves in
org.eclipse.ui.internal.navigator.extensions.StructuredViewerManager.viewerDataMap
by the Platform.... and this cache is a simple Map, that does not
use weak references.
You can try by creating a new Repository from the Git Repositories
view, right-click > add a sub-module. You'll have to make sure
this sub-module contains a pack file (I used git repack after a
commit). Once these conditions are met, you won't be able to
delete that repository from the file system without closing
Eclipse. (closing the Git repositories view is not sufficient.)
Four of the EGit swtbot tests use sub-modules and present this
issue :
-
org.eclipse.egit.ui.view.repositories.GitRepositoriesViewRepoDeletionTest.testDeleteSubmoduleRepository()
- the three tests within package org.eclipse.egit.ui.submodule
What would be the preferred approach: cache the Repository
instances returned by SubmoduleWalk, or clear up the
RepositoryNode when removing them from the Git Repository view?
All in all, I believe that WindowCache relying on the GC to get
rid of the open file descriptors is quite a dangerous thing to do
: we never know what could retain an instance of
"org.eclipse.jgit.lib.Repository" in memory...
Laurent
On 21/03/2013 13:18, Laurent Goubet wrote:
I can
confirm that the failure is random, but when a pack file is held
by the WindowCache, it is locked in that state and nothing seems
to free it: if one deletion fails because of that lock, all will
on that same file, whatever the number of tests that run
in-between. Any test that tries and clear the test folder will
fail with the same error.
Without your proposed change:
Running
GitRepositoriesViewRepoHandlingTest.testSearchDirectoryWithBareRepos(),
the test alone, it will never fail (out of ten runs).
Running GitRepositoriesViewRepoHandlingTest, all tests in the
class, testSearchDirectoryWithBareRepos will fail at random (5
failures out of 6 runs).
With your proposed change, all tests of the class pass, out of 6
runs.
Since it seems like a success, I wanted to use the same kind of
fix for every failure, but there are some less obvious, for
example:
org.eclipse.egit.ui.view.repositories.GitRepositoriesViewRepoDeletionTest.testDeleteSubmoduleRepository()
is particularly aimed at testing repository deletions. Further
down the road, this test uses a
org.eclipse.egit.ui.internal.repository.tree.command.RemoveCommand,
which fails within
deleteRepositoryContent(List<RepositoryNode>, boolean):
repo.close();
FileUtils.delete(repo.getDirectory(),
FileUtils.RECURSIVE | FileUtils.RETRY
| FileUtils.SKIP_MISSING);
the call to "delete" will recursively delete all files within the
repositories but, here too, one of the "pack" files is held open
by the WindowCache, despite the call to "repo.close()" just above.
Laurent
On 21/03/2013 12:30, Robin Stocker wrote:
Hey,
Could you try doing the following change?:
@Test
public void testSearchDirectoryWithBareRepos() throws
Exception {
deleteAllProjects();
+ shutDownRepositories();
clearView();
refreshAndWait();
Regards,
robinst
Laurent Goubet wrote:
A memory profiler helped :).
within the tests,
org.eclipse.egit.ui.view.repositories.GitRepositoriesViewRepoHandlingTest.testSearchDirectoryWithBareRepos()
tries to delete the test directory, directory that contains
the test
repositories in the user home. This recursively deletes all
files it
can find. Deep down, there is a pack file that needs to be
deleted.
This pack file is held open by
org.eclipse.jgit.internal.storage.file.WindowCache . A savage
hack
to close() and removeAll(PackFile) on that particular file
before
the call to "delete" allows the tests to pass.
I do not know these parts of JGit enough to really know what
to do
from there. Any suggestion on how to properly clear the
repository?
Laurent
On 21/03/2013 10:56, Laurent Goubet wrote:
I can't say "always the same" for sure, but since I have from
70 to
90 failures every run, some are failing every time, yes :).
I am currently trying to figure out how I could detect the
leak, but
lack the tools. Currently trying out with a memory profiler if
there
are suspicious input/output streams on that file.
Laurent
On 21/03/2013 10:35, Alex Blewitt wrote:
On 21 Mar 2013, at 09:03, Laurent Goubet wrote:
I usually end up with a lot of errors, most of them because
JGit/EGit
does not manage to remove some of the files from its test
repositories. Disabling my anti-virus does not help. My latest
run
ended up in 79 errors and 1 failure, and left the following
files on
my fs :
Unlike Linux, Windows doesn't let you delete a file if
something else
has it open for read. It's possible that these tests are
exposing a
file handle leak which would not manifest itself on Linux or
MacOSX.
Is it the same tests that are failing each time?
Alex
_______________________________________________
egit-dev mailing list egit-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/egit-dev
_______________________________________________
egit-dev mailing list
egit-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/egit-dev
_______________________________________________
egit-dev mailing list
egit-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/egit-dev
|
