Skip to content

Commit 7c3ecb3

Browse files
drafnelgitster
authored andcommitted
Don't close pack fd when free'ing pack windows
Now that close_one_pack() has been introduced to handle file descriptor pressure, it is not strictly necessary to close the pack file descriptor in unuse_one_window() when we're under memory pressure. Jeff King provided a justification for leaving the pack file open: If you close packfile descriptors, you can run into racy situations where somebody else is repacking and deleting packs, and they go away while you are trying to access them. If you keep a descriptor open, you're fine; they last to the end of the process. If you don't, then they disappear from under you. For normal object access, this isn't that big a deal; we just rescan the packs and retry. But if you are packing yourself (e.g., because you are a pack-objects started by upload-pack for a clone or fetch), it's much harder to recover (and we print some warnings). Let's do so (or uh, not do so). Signed-off-by: Brandon Casey <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 88d0db5 commit 7c3ecb3

File tree

3 files changed

+9
-16
lines changed

3 files changed

+9
-16
lines changed

builtin/pack-objects.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1809,7 +1809,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
18091809
static void try_to_free_from_threads(size_t size)
18101810
{
18111811
read_lock();
1812-
release_pack_memory(size, -1);
1812+
release_pack_memory(size);
18131813
read_unlock();
18141814
}
18151815

git-compat-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ int inet_pton(int af, const char *src, void *dst);
498498
const char *inet_ntop(int af, const void *src, char *dst, size_t size);
499499
#endif
500500

501-
extern void release_pack_memory(size_t, int);
501+
extern void release_pack_memory(size_t);
502502

503503
typedef void (*try_to_free_t)(size_t);
504504
extern try_to_free_t set_try_to_free_routine(try_to_free_t);

sha1_file.c

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ static void scan_windows(struct packed_git *p,
605605
}
606606
}
607607

608-
static int unuse_one_window(struct packed_git *current, int keep_fd)
608+
static int unuse_one_window(struct packed_git *current)
609609
{
610610
struct packed_git *p, *lru_p = NULL;
611611
struct pack_window *lru_w = NULL, *lru_l = NULL;
@@ -619,26 +619,19 @@ static int unuse_one_window(struct packed_git *current, int keep_fd)
619619
pack_mapped -= lru_w->len;
620620
if (lru_l)
621621
lru_l->next = lru_w->next;
622-
else {
622+
else
623623
lru_p->windows = lru_w->next;
624-
if (!lru_p->windows && lru_p->pack_fd != -1
625-
&& lru_p->pack_fd != keep_fd) {
626-
close(lru_p->pack_fd);
627-
pack_open_fds--;
628-
lru_p->pack_fd = -1;
629-
}
630-
}
631624
free(lru_w);
632625
pack_open_windows--;
633626
return 1;
634627
}
635628
return 0;
636629
}
637630

638-
void release_pack_memory(size_t need, int fd)
631+
void release_pack_memory(size_t need)
639632
{
640633
size_t cur = pack_mapped;
641-
while (need >= (cur - pack_mapped) && unuse_one_window(NULL, fd))
634+
while (need >= (cur - pack_mapped) && unuse_one_window(NULL))
642635
; /* nothing */
643636
}
644637

@@ -649,7 +642,7 @@ void *xmmap(void *start, size_t length,
649642
if (ret == MAP_FAILED) {
650643
if (!length)
651644
return NULL;
652-
release_pack_memory(length, fd);
645+
release_pack_memory(length);
653646
ret = mmap(start, length, prot, flags, fd, offset);
654647
if (ret == MAP_FAILED)
655648
die_errno("Out of memory? mmap failed");
@@ -961,7 +954,7 @@ unsigned char *use_pack(struct packed_git *p,
961954
win->len = (size_t)len;
962955
pack_mapped += win->len;
963956
while (packed_git_limit < pack_mapped
964-
&& unuse_one_window(p, p->pack_fd))
957+
&& unuse_one_window(p))
965958
; /* nothing */
966959
win->base = xmmap(NULL, win->len,
967960
PROT_READ, MAP_PRIVATE,
@@ -1007,7 +1000,7 @@ static struct packed_git *alloc_packed_git(int extra)
10071000

10081001
static void try_to_free_pack_memory(size_t size)
10091002
{
1010-
release_pack_memory(size, -1);
1003+
release_pack_memory(size);
10111004
}
10121005

10131006
struct packed_git *add_packed_git(const char *path, int path_len, int local)

0 commit comments

Comments
 (0)