Skip to content

Commit cf054f8

Browse files
gitstergitster
committed
Merge branch 'tb/commit-graph-fd-exhaustion-fix'
The commit-graph code exhausted file descriptors easily when it does not have to. * tb/commit-graph-fd-exhaustion-fix: commit-graph: close descriptors after mmap commit-graph.c: gracefully handle file descriptor exhaustion t/test-lib.sh: make ULIMIT_FILE_DESCRIPTORS available to tests commit-graph.c: don't use discarded graph_name in error
2 parents 6a1c17d + c882853 commit cf054f8

File tree

6 files changed

+33
-29
lines changed

6 files changed

+33
-29
lines changed

‎commit-graph.c‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ static uint8_t oid_version(void)
6969
static struct commit_graph *alloc_commit_graph(void)
7070
{
7171
struct commit_graph *g = xcalloc(1, sizeof(*g));
72-
g->graph_fd = -1;
7372

7473
return g;
7574
}
@@ -123,14 +122,13 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st,
123122
return NULL;
124123
}
125124
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
126-
ret = parse_commit_graph(graph_map, fd, graph_size);
125+
close(fd);
126+
ret = parse_commit_graph(graph_map, graph_size);
127127

128128
if (ret)
129129
ret->odb = odb;
130-
else {
130+
else
131131
munmap(graph_map, graph_size);
132-
close(fd);
133-
}
134132

135133
return ret;
136134
}
@@ -165,8 +163,7 @@ static int verify_commit_graph_lite(struct commit_graph *g)
165163
return 0;
166164
}
167165

168-
struct commit_graph *parse_commit_graph(void *graph_map, int fd,
169-
size_t graph_size)
166+
struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size)
170167
{
171168
const unsigned char *data, *chunk_lookup;
172169
uint32_t i;
@@ -209,7 +206,6 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
209206

210207
graph->hash_len = the_hash_algo->rawsz;
211208
graph->num_chunks = *(unsigned char*)(data + 6);
212-
graph->graph_fd = fd;
213209
graph->data = graph_map;
214210
graph->data_len = graph_size;
215211

@@ -1397,7 +1393,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
13971393

13981394
fd = git_mkstemp_mode(ctx->graph_name, 0444);
13991395
if (fd < 0) {
1400-
error(_("unable to create '%s'"), ctx->graph_name);
1396+
error(_("unable to create temporary graph layer"));
14011397
return -1;
14021398
}
14031399

@@ -1602,8 +1598,8 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
16021598
free(old_graph_name);
16031599
}
16041600

1605-
ALLOC_ARRAY(ctx->commit_graph_filenames_after, ctx->num_commit_graphs_after);
1606-
ALLOC_ARRAY(ctx->commit_graph_hash_after, ctx->num_commit_graphs_after);
1601+
CALLOC_ARRAY(ctx->commit_graph_filenames_after, ctx->num_commit_graphs_after);
1602+
CALLOC_ARRAY(ctx->commit_graph_hash_after, ctx->num_commit_graphs_after);
16071603

16081604
for (i = 0; i < ctx->num_commit_graphs_after &&
16091605
i < ctx->num_commit_graphs_before; i++)
@@ -2125,10 +2121,9 @@ void free_commit_graph(struct commit_graph *g)
21252121
{
21262122
if (!g)
21272123
return;
2128-
if (g->graph_fd >= 0) {
2124+
if (g->data) {
21292125
munmap((void *)g->data, g->data_len);
21302126
g->data = NULL;
2131-
close(g->graph_fd);
21322127
}
21332128
free(g->filename);
21342129
free(g);

‎commit-graph.h‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ struct tree *get_commit_tree_in_graph(struct repository *r,
4040
const struct commit *c);
4141

4242
struct commit_graph {
43-
int graph_fd;
44-
4543
const unsigned char *data;
4644
size_t data_len;
4745

@@ -66,8 +64,7 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st,
6664
struct object_directory *odb);
6765
struct commit_graph *read_commit_graph_one(struct repository *r,
6866
struct object_directory *odb);
69-
struct commit_graph *parse_commit_graph(void *graph_map, int fd,
70-
size_t graph_size);
67+
struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size);
7168

7269
/*
7370
* Return 1 if and only if the repository has a commit-graph

‎fuzz-commit-graph.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
#include "commit-graph.h"
22
#include "repository.h"
33

4-
struct commit_graph *parse_commit_graph(void *graph_map, int fd,
5-
size_t graph_size);
4+
struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size);
65

76
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
87

@@ -11,7 +10,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
1110
struct commit_graph *g;
1211

1312
initialize_the_repository();
14-
g = parse_commit_graph((void *)data, -1, size);
13+
g = parse_commit_graph((void *)data, size);
1514
repo_clear(the_repository);
1615
free(g);
1716

‎t/t1400-update-ref.sh‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,15 +1354,6 @@ test_expect_success 'fails with duplicate ref update via symref' '
13541354
test_cmp expect actual
13551355
'
13561356

1357-
run_with_limited_open_files () {
1358-
(ulimit -n 32 && "$@")
1359-
}
1360-
1361-
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS '
1362-
test_have_prereq !MINGW,!CYGWIN &&
1363-
run_with_limited_open_files true
1364-
'
1365-
13661357
test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
13671358
(
13681359
for i in $(test_seq 33)

‎t/t5324-split-commit-graph.sh‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,4 +387,17 @@ test_expect_success '--split=replace replaces the chain' '
387387
graph_read_expect 2
388388
'
389389

390+
test_expect_success ULIMIT_FILE_DESCRIPTORS 'handles file descriptor exhaustion' '
391+
git init ulimit &&
392+
(
393+
cd ulimit &&
394+
for i in $(test_seq 64)
395+
do
396+
test_commit $i &&
397+
test_might_fail run_with_limited_open_files git commit-graph write \
398+
--split=no-merge --reachable || return 1
399+
done
400+
)
401+
'
402+
390403
test_done

‎t/test-lib.sh‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,6 +1666,15 @@ test_lazy_prereq ULIMIT_STACK_SIZE '
16661666
run_with_limited_stack true
16671667
'
16681668

1669+
run_with_limited_open_files () {
1670+
(ulimit -n 32 && "$@")
1671+
}
1672+
1673+
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS '
1674+
test_have_prereq !MINGW,!CYGWIN &&
1675+
run_with_limited_open_files true
1676+
'
1677+
16691678
build_option () {
16701679
git version --build-options |
16711680
sed -ne "s/^$1: //p"

0 commit comments

Comments
 (0)