On Tue, Jun 29, 2021 at 08:11:05AM +0000, Eric Wong wrote:

Since use_pack() can already safely munmap packs to respect
core.packedGitLimit, attempt to gracefully handle ENOMEM
errors the same way by unmapping a window and retrying.

This benefits unprivileged users who lack permissions to raise
the `sys.vm.max_map_count' sysctl and/or RLIMIT_DATA resource
limit.

I've also verified it is safe to release a pack here by
unconditionally calling unuse_one_window() before
xmmap_gently():

	--- a/packfile.c
	+++ b/packfile.c
	@@ -649,6 +649,7 @@ unsigned char *use_pack(struct packed_git *p,
					&& unuse_one_window(p))
					; /* nothing */
				do {
	+				unuse_one_window(p);
					win->base = xmmap_gently(NULL, win->len,
						PROT_READ, MAP_PRIVATE,
						p->pack_fd, win->offset);

I don't find that test-diff all that compelling, because we don't know
which window will get unused. I.e., if there is one that will get racily
unused, we might not hit it. I think it would be a lot more interesting
for finding problems if it did:

  while (unuse_one_window(p)) ;

to clear them all.

That said, I think this must be obviously correct because the code above
will potentially have just called unuse_one_window(p) already. So at
least if not obviously correct, no more buggy than the previous code. :)

-Peff