git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Max Kirillov <max@max630.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Max Kirillov <max@max630.net>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	git@vger.kernel.org
Subject: [PATCH/RFC 1/2] sha1_file: close all pack files after running
Date: Thu,  1 Oct 2015 06:29:22 +0300	[thread overview]
Message-ID: <1443670163-31193-2-git-send-email-max@max630.net> (raw)
In-Reply-To: <1443670163-31193-1-git-send-email-max@max630.net>

When a builtin has done its job, but waits for pager or not waited
by its caller and still hanging it keeps pack files opened.
This can cause a number of issues, for example on Windows git gc
cannot remove the packs.

Fix this by explicitly closing all pack files and unmapping memory
from the packs after running builtin. Do not die() if the memory region
is still used though, just print a warning, because failure to close
a file should not prevent the currently running program from finishing
its task.

Signed-off-by: Max Kirillov <max@max630.net>
---
 cache.h     |  1 +
 git.c       |  2 ++
 sha1_file.c | 32 +++++++++++++++++++++++++++++---
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 79066e5..153bc46 100644
--- a/cache.h
+++ b/cache.h
@@ -1279,6 +1279,7 @@ extern void unuse_pack(struct pack_window **);
 extern void free_pack_by_name(const char *);
 extern void clear_delta_base_cache(void);
 extern struct packed_git *add_packed_git(const char *, int, int);
+extern void close_all_packs(void);
 
 /*
  * Return the SHA-1 of the nth object within the specified packfile.
diff --git a/git.c b/git.c
index 5feba41..ad34680 100644
--- a/git.c
+++ b/git.c
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "cache.h"
 #include "exec_cmd.h"
 #include "help.h"
 #include "run-command.h"
@@ -348,6 +349,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	trace_argv_printf(argv, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
+	close_all_packs();
 	if (status)
 		return status;
 
diff --git a/sha1_file.c b/sha1_file.c
index 08302f5..62f1dad 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -773,20 +773,28 @@ void *xmmap(void *start, size_t length,
 	return ret;
 }
 
-void close_pack_windows(struct packed_git *p)
+static int close_pack_windows_nodie(struct packed_git *p)
 {
 	while (p->windows) {
 		struct pack_window *w = p->windows;
 
 		if (w->inuse_cnt)
-			die("pack '%s' still has open windows to it",
-			    p->pack_name);
+			return 1;
+
 		munmap(w->base, w->len);
 		pack_mapped -= w->len;
 		pack_open_windows--;
 		p->windows = w->next;
 		free(w);
 	}
+
+	return 0;
+}
+
+void close_pack_windows(struct packed_git *p)
+{
+	if (close_pack_windows_nodie(p))
+		die("pack '%s' still has open windows to it", p->pack_name);
 }
 
 /*
@@ -866,6 +874,24 @@ static int close_one_pack(void)
 	return 0;
 }
 
+void close_all_packs(void)
+{
+	struct packed_git *p = NULL;
+
+	for (p = packed_git; p; p = p->next) {
+		if (close_pack_windows_nodie(p))
+			warning("pack '%s' still has open windows to it", p->pack_name);
+
+		if (p->pack_fd != -1) {
+			if (close(p->pack_fd) != 0)
+				warning("close(%s) failed: %d", p->pack_name, errno);
+			p->pack_fd = -1;
+		}
+
+		close_pack_index(p);
+	}
+}
+
 void unuse_pack(struct pack_window **w_cursor)
 {
 	struct pack_window *w = *w_cursor;
-- 
2.3.4.2801.g3d0809b

  reply	other threads:[~2015-10-01  3:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-28 19:44 [PATCH] clone --dissociate: avoid locking pack files Johannes Schindelin
2015-09-30 19:28 ` Max Kirillov
2015-09-30 19:42   ` Junio C Hamano
2015-10-01  3:29     ` [PATCH/RFC 0/2] close packs files when they are not needed Max Kirillov
2015-10-01  3:29       ` Max Kirillov [this message]
2015-10-02 10:05         ` [PATCH/RFC 1/2] sha1_file: close all pack files after running Johannes Schindelin
2015-10-02 10:13           ` Johannes Schindelin
2015-10-02 19:21             ` Max Kirillov
2015-10-04 14:53               ` Johannes Schindelin
2015-10-05  4:57                 ` Max Kirillov
2015-10-05  9:03                   ` Johannes Schindelin
2015-10-02 19:06           ` Max Kirillov
2015-10-02 20:06             ` Max Kirillov
2015-10-01  3:29       ` [PATCH/RFC 2/2] sha1_file: set packfile to O_CLOEXEC at open Max Kirillov
2015-10-02 10:08         ` Johannes Schindelin
2015-10-01  4:39   ` [PATCH] clone --dissociate: avoid locking pack files Max Kirillov
2015-10-05 18:32     ` Johannes Schindelin
2015-10-05 20:29 ` [PATCH v2 0/4] Fix locking issues on Windows with `git clone --dissociate` Johannes Schindelin
2015-10-05 20:29   ` [PATCH v2 1/4] Demonstrate a Windows file locking issue " Johannes Schindelin
2015-10-05 20:30   ` [PATCH v2 2/4] Consolidate code to close a pack's file descriptor Johannes Schindelin
2015-10-05 20:57     ` Junio C Hamano
2015-10-05 21:52       ` Johannes Schindelin
2015-10-05 22:15         ` Junio C Hamano
2015-10-06 13:42           ` Johannes Schindelin
2015-10-05 20:33   ` [PATCH v2 3/4] Add a function to release all packs Johannes Schindelin
2015-10-05 20:33   ` [PATCH v2 4/4] clone --dissociate: avoid locking pack files Johannes Schindelin
2015-10-05 21:00     ` Junio C Hamano
2015-10-06 13:17 ` [PATCH v3 0/4] Fix locking issues on Windows with `git clone --dissociate` Johannes Schindelin
2015-10-06 13:18   ` [PATCH v3 1/4] Demonstrate a Windows file locking issue " Johannes Schindelin
2015-10-06 13:18   ` [PATCH v3 2/4] Consolidate code to close a pack's file descriptor Johannes Schindelin
2015-10-06 13:18   ` [PATCH v3 3/4] Add a function to release all packs Johannes Schindelin
2015-10-07 17:49     ` Junio C Hamano
2015-10-08 19:10       ` Johannes Schindelin
2015-10-06 13:18   ` [PATCH v3 4/4] clone --dissociate: avoid locking pack files Johannes Schindelin
2015-10-11 10:45   ` [PATCH v3 0/4] Fix locking issues on Windows with `git clone --dissociate` Max Kirillov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1443670163-31193-2-git-send-email-max@max630.net \
    --to=max@max630.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).