P.S.: to elaborate, something like this
solves the issue:
SubmoduleWalk walk = SubmoduleWalk.forIndex(node.getRepository());
while (walk.next()) {
Repository subRepo = walk.getRepository();
- if (subRepo != null)
- subRepo.close()
+
if (subRepo != null) {
+ children.add(new RepositoryNode(node,
repositoryCache.lookupRepository(subRepo.getDirectory())
));
+ subRepo.close() ;
+ }
On 21/03/2013 16:44, Laurent Goubet wrote:
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
|