diff options
author | Junio C Hamano <gitster@pobox.com> | 2020-05-01 13:39:53 -0700 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2020-05-01 13:39:53 -0700 |
commit | cf054f817a30cf4a6531548f52cd7d5cbed6f4fc (patch) | |
tree | 1a22fdf3be94217a1427f8728dc1bf3a0e25692e | |
parent | Merge branch 'tb/commit-graph-split-strategy' (diff) | |
parent | commit-graph: close descriptors after mmap (diff) | |
download | tgif-cf054f817a30cf4a6531548f52cd7d5cbed6f4fc.tar.xz |
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
-rw-r--r-- | commit-graph.c | 21 | ||||
-rw-r--r-- | commit-graph.h | 5 | ||||
-rw-r--r-- | fuzz-commit-graph.c | 5 | ||||
-rwxr-xr-x | t/t1400-update-ref.sh | 9 | ||||
-rwxr-xr-x | t/t5324-split-commit-graph.sh | 13 | ||||
-rw-r--r-- | t/test-lib.sh | 9 |
6 files changed, 33 insertions, 29 deletions
diff --git a/commit-graph.c b/commit-graph.c index d11c836e1f..e9b3c5d561 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -69,7 +69,6 @@ static uint8_t oid_version(void) static struct commit_graph *alloc_commit_graph(void) { struct commit_graph *g = xcalloc(1, sizeof(*g)); - g->graph_fd = -1; return g; } @@ -123,14 +122,13 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st, return NULL; } graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); - ret = parse_commit_graph(graph_map, fd, graph_size); + close(fd); + ret = parse_commit_graph(graph_map, graph_size); if (ret) ret->odb = odb; - else { + else munmap(graph_map, graph_size); - close(fd); - } return ret; } @@ -165,8 +163,7 @@ static int verify_commit_graph_lite(struct commit_graph *g) return 0; } -struct commit_graph *parse_commit_graph(void *graph_map, int fd, - size_t graph_size) +struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size) { const unsigned char *data, *chunk_lookup; uint32_t i; @@ -209,7 +206,6 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, graph->hash_len = the_hash_algo->rawsz; graph->num_chunks = *(unsigned char*)(data + 6); - graph->graph_fd = fd; graph->data = graph_map; graph->data_len = graph_size; @@ -1397,7 +1393,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) fd = git_mkstemp_mode(ctx->graph_name, 0444); if (fd < 0) { - error(_("unable to create '%s'"), ctx->graph_name); + error(_("unable to create temporary graph layer")); return -1; } @@ -1602,8 +1598,8 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) free(old_graph_name); } - ALLOC_ARRAY(ctx->commit_graph_filenames_after, ctx->num_commit_graphs_after); - ALLOC_ARRAY(ctx->commit_graph_hash_after, ctx->num_commit_graphs_after); + CALLOC_ARRAY(ctx->commit_graph_filenames_after, ctx->num_commit_graphs_after); + CALLOC_ARRAY(ctx->commit_graph_hash_after, ctx->num_commit_graphs_after); for (i = 0; i < ctx->num_commit_graphs_after && i < ctx->num_commit_graphs_before; i++) @@ -2125,10 +2121,9 @@ void free_commit_graph(struct commit_graph *g) { if (!g) return; - if (g->graph_fd >= 0) { + if (g->data) { munmap((void *)g->data, g->data_len); g->data = NULL; - close(g->graph_fd); } free(g->filename); free(g); diff --git a/commit-graph.h b/commit-graph.h index 98ef121924..1254eae948 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -40,8 +40,6 @@ struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c); struct commit_graph { - int graph_fd; - const unsigned char *data; size_t data_len; @@ -66,8 +64,7 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st, struct object_directory *odb); struct commit_graph *read_commit_graph_one(struct repository *r, struct object_directory *odb); -struct commit_graph *parse_commit_graph(void *graph_map, int fd, - size_t graph_size); +struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size); /* * Return 1 if and only if the repository has a commit-graph diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c index 0157acbf2e..9fd1c04edd 100644 --- a/fuzz-commit-graph.c +++ b/fuzz-commit-graph.c @@ -1,8 +1,7 @@ #include "commit-graph.h" #include "repository.h" -struct commit_graph *parse_commit_graph(void *graph_map, int fd, - size_t graph_size); +struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size); int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size); @@ -11,7 +10,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) struct commit_graph *g; initialize_the_repository(); - g = parse_commit_graph((void *)data, -1, size); + g = parse_commit_graph((void *)data, size); repo_clear(the_repository); free(g); diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 48d0d42afd..e1197ac818 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -1354,15 +1354,6 @@ test_expect_success 'fails with duplicate ref update via symref' ' test_cmp expect actual ' -run_with_limited_open_files () { - (ulimit -n 32 && "$@") -} - -test_lazy_prereq ULIMIT_FILE_DESCRIPTORS ' - test_have_prereq !MINGW,!CYGWIN && - run_with_limited_open_files true -' - test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' ' ( for i in $(test_seq 33) diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index 3378ec54cd..4146b82031 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -387,4 +387,17 @@ test_expect_success '--split=replace replaces the chain' ' graph_read_expect 2 ' +test_expect_success ULIMIT_FILE_DESCRIPTORS 'handles file descriptor exhaustion' ' + git init ulimit && + ( + cd ulimit && + for i in $(test_seq 64) + do + test_commit $i && + test_might_fail run_with_limited_open_files git commit-graph write \ + --split=no-merge --reachable || return 1 + done + ) +' + test_done diff --git a/t/test-lib.sh b/t/test-lib.sh index 13ba00d508..1b221951a8 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1666,6 +1666,15 @@ test_lazy_prereq ULIMIT_STACK_SIZE ' run_with_limited_stack true ' +run_with_limited_open_files () { + (ulimit -n 32 && "$@") +} + +test_lazy_prereq ULIMIT_FILE_DESCRIPTORS ' + test_have_prereq !MINGW,!CYGWIN && + run_with_limited_open_files true +' + build_option () { git version --build-options | sed -ne "s/^$1: //p" |