Eliminate file handle leaks

Running the SWTBot test suite on a Windows environment highlights
insidious file handle leaks caused by JGit's WindowCache which keeps
".pack" files open for reading and thus locked for deletion until the
repository is properly closed, but EGit might not keep a reference
towards this repository in its cache, "forgetting" to close it when
needed. This is mostly true for sub-modules.

This patch also gets rid of other random errors, sometimes due to a
.project not properly removed.

Bug: 404121
Change-Id: I3a596fcabdb5225e9042e1ffae6b054c48d72d54
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

8 files changed, 50 insertions, 10 deletions

diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/SubmoduleAddOperation.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/SubmoduleAddOperation.java
index 5d1860db13..1f7f710901 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/SubmoduleAddOperation.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/SubmoduleAddOperation.java
@@ -1,5 +1,5 @@
 /******************************************************************************
- *  Copyright (c) 2012 GitHub Inc.
+ *  Copyright (c) 2012, 2013 GitHub Inc and others.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  *  Contributors:
  *    Kevin Sawicki (GitHub Inc.) - initial API and implementation
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *****************************************************************************/
 package org.eclipse.egit.core.op;
 
@@ -57,8 +58,11 @@ public class SubmoduleAddOperation implements IEGitOperation {
 				add.setPath(path);
 				add.setURI(uri);
 				try {
-					if (add.call() != null)
+					Repository subRepo = add.call();
+					if (subRepo != null) {
+						subRepo.close();
 						repo.notifyIndexChanged();
+					}
 				} catch (GitAPIException e) {
 					throw new TeamException(e.getLocalizedMessage(),
 							e.getCause());
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleSyncTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleSyncTest.java
index 2a73d40191..f8972a72b2 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleSyncTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleSyncTest.java
@@ -1,5 +1,5 @@
 /******************************************************************************
- *  Copyright (c) 2012 GitHub Inc and others.
+ *  Copyright (c) 2012, 2013 GitHub Inc and others.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  *  Contributors:
  *    Kevin Sawicki (GitHub Inc.) - initial API and implementation
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *****************************************************************************/
 package org.eclipse.egit.ui.submodule;
 
@@ -70,6 +71,7 @@ public class SubmoduleSyncTest extends GitRepositoriesViewTestBase {
 		command.setURI(uri);
 		Repository subRepo = command.call();
 		assertNotNull(subRepo);
+		subRepo.close();
 
 		String newUri = "git://server/repo.git";
 		File modulesFile = new File(repo.getWorkTree(),
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleUpdateTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleUpdateTest.java
index 8a118b16cd..a0815bdd67 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleUpdateTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/submodule/SubmoduleUpdateTest.java
@@ -1,5 +1,5 @@
 /******************************************************************************
- *  Copyright (c) 2012 GitHub Inc and others.
+ *  Copyright (c) 2012, 2013 GitHub Inc and others.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  *  Contributors:
  *    Kevin Sawicki (GitHub Inc.) - initial API and implementation
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *****************************************************************************/
 package org.eclipse.egit.ui.submodule;
 
@@ -73,6 +74,7 @@ public class SubmoduleUpdateTest extends GitRepositoriesViewTestBase {
 		command.setURI(uri);
 		Repository subRepo = command.call();
 		assertNotNull(subRepo);
+		subRepo.close();
 
 		Ref head = subRepo.getRef(Constants.HEAD);
 		assertNotNull(head);
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
index 1c961957f7..4aa74d218a 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, Matthias Sohn <matthias.sohn@sap.com> and others.
+ * Copyright (c) 2012, 2013 Matthias Sohn <matthias.sohn@sap.com> and others.
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -98,6 +98,7 @@ public class GitRepositoriesViewRepoDeletionTest extends
 		command.setURI(uri);
 		Repository subRepo = command.call();
 		assertNotNull(subRepo);
+		subRepo.close();
 
 		refreshAndWait();
 
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoHandlingTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoHandlingTest.java
index 7ec0e513aa..9209a22b18 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoHandlingTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoHandlingTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2012 SAP AG and others.
+ * Copyright (c) 2010, 2013 SAP AG and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *    Mathias Kinzler (SAP AG) - initial implementation
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *******************************************************************************/
 package org.eclipse.egit.ui.view.repositories;
 
@@ -365,6 +366,7 @@ public class GitRepositoriesViewRepoHandlingTest extends
 	@Test
 	public void testSearchDirectoryWithBareRepos() throws Exception {
 		deleteAllProjects();
+		shutDownRepositories();
 		clearView();
 		refreshAndWait();
 		assertEmpty();
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/synchronize/AbstractSynchronizeViewTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/synchronize/AbstractSynchronizeViewTest.java
index 3233a89506..d2a8766497 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/synchronize/AbstractSynchronizeViewTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/synchronize/AbstractSynchronizeViewTest.java
@@ -96,6 +96,14 @@ public abstract class AbstractSynchronizeViewTest extends
 		syncView.close();
 	}
 
+	@After
+	public void deleteEmptyProject() throws Exception {
+		IProject prj = ResourcesPlugin.getWorkspace().getRoot()
+				.getProject(EMPTY_PROJECT);
+		if (prj.exists())
+			prj.delete(false, false, null);
+	}
+
 	@Before
 	public void setupRepository() throws Exception {
 		repositoryFile = createProjectAndCommitToRepository();
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
index fc4ada3075..d5458c88f5 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010 SAP AG.
+ * Copyright (c) 2010, 2013 SAP AG and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *    Mathias Kinzler (SAP AG) - initial implementation
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *******************************************************************************/
 package org.eclipse.egit.ui.internal.repository;
 
@@ -434,8 +435,12 @@ public class RepositoriesViewContentProvider implements ITreeContentProvider,
 						.getRepository());
 				while (walk.next()) {
 					Repository subRepo = walk.getRepository();
-					if (subRepo != null)
-						children.add(new RepositoryNode(node, subRepo));
+					if (subRepo != null) {
+						final Repository cachedRepo = repositoryCache
+								.lookupRepository(subRepo.getDirectory());
+						subRepo.close();
+						children.add(new RepositoryNode(node, cachedRepo));
+					}
 				}
 			} catch (IOException e) {
 				handleException(e, node);
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
index e7b9307757..d49ca642c4 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2012 SAP AG and others.
+ * Copyright (c) 2010, 2013 SAP AG and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,6 +8,7 @@
  * Contributors:
  *    Mathias Kinzler (SAP AG) - initial implementation
  *    Daniel Megert <daniel_megert@ch.ibm.com> - Delete empty working directory
+ *    Laurent Goubet <laurent.goubet@obeo.fr - 404121
  *******************************************************************************/
 package org.eclipse.egit.ui.internal.repository.tree.command;
 
@@ -31,6 +32,7 @@ import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.egit.core.RepositoryCache;
 import org.eclipse.egit.ui.Activator;
 import org.eclipse.egit.ui.JobFamilies;
 import org.eclipse.egit.ui.internal.UIText;
@@ -45,6 +47,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.submodule.SubmoduleWalk;
 import org.eclipse.jgit.treewalk.EmptyTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
@@ -218,6 +221,19 @@ public class RemoveCommand extends
 					}
 			}
 			repo.close();
+
+			SubmoduleWalk walk = SubmoduleWalk.forIndex(repo);
+			while (walk.next()) {
+				Repository subRepo = walk.getRepository();
+				if (subRepo != null) {
+					final RepositoryCache cache = org.eclipse.egit.core.Activator
+							.getDefault().getRepositoryCache();
+					cache.lookupRepository(subRepo.getDirectory()).close();
+					subRepo.close();
+				}
+			}
+			walk.release();
+
 			FileUtils.delete(repo.getDirectory(),
 					FileUtils.RECURSIVE | FileUtils.RETRY
 							| FileUtils.SKIP_MISSING);