From 24ac4ff190a96de47fab73d81c257f089c0e2020 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 3 Nov 2016 08:32:19 -0400 Subject: [PATCH] deltas: Only keep one file open at a time during compilation Otherwise it's possible for us to exhaust available file descriptors or (on 32 bit) run up against mmap limits. In the rollsum case, we didn't need to hold open the "from" object at all. And in the bsdiff case, we weren't even looking at either of the files until we started processing. Also, while we have the patient open, switch to using O_TMPFILE if available. Closes: #567 Approved by: giuseppe --- .../ostree-repo-static-delta-compilation.c | 111 +++++++----------- 1 file changed, 45 insertions(+), 66 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index fef204c7..286c74e1 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -32,6 +32,7 @@ #include "ostree-diff.h" #include "ostree-rollsum.h" #include "otutil.h" +#include "libglnx.h" #include "ostree-varint.h" #include "bsdiff/bsdiff.h" @@ -413,15 +414,11 @@ process_one_object (OstreeRepo *repo, typedef struct { char *from_checksum; - GBytes *tmp_from; - GBytes *tmp_to; } ContentBsdiff; typedef struct { char *from_checksum; OstreeRollsumMatches *matches; - GBytes *tmp_from; - GBytes *tmp_to; } ContentRollsum; static void @@ -429,8 +426,6 @@ content_rollsums_free (ContentRollsum *rollsum) { g_free (rollsum->from_checksum); _ostree_rollsum_matches_free (rollsum->matches); - g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); - g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); g_free (rollsum); } @@ -438,8 +433,6 @@ static void content_bsdiffs_free (ContentBsdiff *bsdiff) { g_free (bsdiff->from_checksum); - g_clear_pointer (&bsdiff->tmp_from, g_bytes_unref); - g_clear_pointer (&bsdiff->tmp_to, g_bytes_unref); g_free (bsdiff); } @@ -450,50 +443,40 @@ static gboolean get_unpacked_unlinked_content (OstreeRepo *repo, const char *checksum, GBytes **out_content, - GFileInfo **out_finfo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autofree char *tmpname = g_strdup ("/var/tmp/tmpostree-deltaobj-XXXXXX"); + g_autofree char *tmpname = NULL; glnx_fd_close int fd = -1; g_autoptr(GBytes) ret_content = NULL; g_autoptr(GInputStream) istream = NULL; - g_autoptr(GFileInfo) ret_finfo = NULL; g_autoptr(GOutputStream) out = NULL; - fd = g_mkstemp (tmpname); - if (fd == -1) - { - glnx_set_error_from_errno (error); - goto out; - } - /* Doesn't need a name */ - (void) unlink (tmpname); + if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, "/tmp", O_RDWR | O_CLOEXEC, + &fd, &tmpname, error)) + return FALSE; + /* We don't need the file name */ + if (tmpname) + (void) unlinkat (AT_FDCWD, tmpname, 0); - if (!ostree_repo_load_file (repo, checksum, &istream, &ret_finfo, NULL, + if (!ostree_repo_load_file (repo, checksum, &istream, NULL, NULL, cancellable, error)) - goto out; + return FALSE; - g_assert (g_file_info_get_file_type (ret_finfo) == G_FILE_TYPE_REGULAR); - out = g_unix_output_stream_new (fd, FALSE); if (g_output_stream_splice (out, istream, G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET, cancellable, error) < 0) - goto out; - - { GMappedFile *mfile = g_mapped_file_new_from_fd (fd, FALSE, error); + return FALSE; + + { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (fd, FALSE, error); if (!mfile) - goto out; + return FALSE; ret_content = g_mapped_file_get_bytes (mfile); - g_mapped_file_unref (mfile); } - ret = TRUE; if (out_content) *out_content = g_steal_pointer (&ret_content); - out: - return ret; + return TRUE; } static gboolean @@ -506,22 +489,20 @@ try_content_bsdiff (OstreeRepo *repo, GError **error) { gboolean ret = FALSE; - g_autoptr(GBytes) tmp_from = NULL; - g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) from_finfo = NULL; g_autoptr(GFileInfo) to_finfo = NULL; ContentBsdiff *ret_bsdiff = NULL; *out_bsdiff = NULL; - if (!get_unpacked_unlinked_content (repo, from, &tmp_from, &from_finfo, - cancellable, error)) - goto out; - if (!get_unpacked_unlinked_content (repo, to, &tmp_to, &to_finfo, - cancellable, error)) - goto out; + if (!ostree_repo_load_file (repo, from, NULL, &from_finfo, NULL, + cancellable, error)) + return FALSE; + if (!ostree_repo_load_file (repo, to, NULL, &to_finfo, NULL, + cancellable, error)) + return FALSE; - if (g_bytes_get_size (tmp_to) + g_bytes_get_size (tmp_from) > max_bsdiff_size_bytes) + if (g_file_info_get_size (to_finfo) + g_file_info_get_size (from_finfo) > max_bsdiff_size_bytes) { ret = TRUE; goto out; @@ -529,8 +510,6 @@ try_content_bsdiff (OstreeRepo *repo, ret_bsdiff = g_new0 (ContentBsdiff, 1); ret_bsdiff->from_checksum = g_strdup (from); - ret_bsdiff->tmp_from = tmp_from; tmp_from = NULL; - ret_bsdiff->tmp_to = tmp_to; tmp_to = NULL; ret = TRUE; if (out_bsdiff) @@ -551,8 +530,6 @@ try_content_rollsum (OstreeRepo *repo, gboolean ret = FALSE; g_autoptr(GBytes) tmp_from = NULL; g_autoptr(GBytes) tmp_to = NULL; - g_autoptr(GFileInfo) from_finfo = NULL; - g_autoptr(GFileInfo) to_finfo = NULL; OstreeRollsumMatches *matches = NULL; ContentRollsum *ret_rollsum = NULL; @@ -561,11 +538,9 @@ try_content_rollsum (OstreeRepo *repo, /* Load the content objects, splice them to uncompressed temporary files that * we can just mmap() and seek around in conveniently. */ - if (!get_unpacked_unlinked_content (repo, from, &tmp_from, &from_finfo, - cancellable, error)) + if (!get_unpacked_unlinked_content (repo, from, &tmp_from, cancellable, error)) goto out; - if (!get_unpacked_unlinked_content (repo, to, &tmp_to, &to_finfo, - cancellable, error)) + if (!get_unpacked_unlinked_content (repo, to, &tmp_to, cancellable, error)) goto out; matches = _ostree_compute_rollsum_matches (tmp_from, tmp_to); @@ -592,10 +567,8 @@ try_content_rollsum (OstreeRepo *repo, ret_rollsum = g_new0 (ContentRollsum, 1); ret_rollsum->from_checksum = g_strdup (from); - ret_rollsum->matches = matches; matches = NULL; - ret_rollsum->tmp_from = tmp_from; tmp_from = NULL; - ret_rollsum->tmp_to = tmp_to; tmp_to = NULL; - + ret_rollsum->matches = g_steal_pointer (&matches); + ret = TRUE; if (out_rollsum) *out_rollsum = g_steal_pointer (&ret_rollsum); @@ -651,7 +624,7 @@ process_one_rollsum (OstreeRepo *repo, { gboolean ret = FALSE; guint64 content_size; - g_autoptr(GInputStream) content_stream = NULL; + g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) content_finfo = NULL; g_autoptr(GVariant) content_xattrs = NULL; OstreeStaticDeltaPartBuilder *current_part = *current_part_val; @@ -665,9 +638,13 @@ process_one_rollsum (OstreeRepo *repo, *current_part_val = current_part = allocate_part (builder); } - tmp_to_buf = g_bytes_get_data (rollsum->tmp_to, &tmp_to_len); + if (!get_unpacked_unlinked_content (repo, to_checksum, &tmp_to, + cancellable, error)) + goto out; - if (!ostree_repo_load_file (repo, to_checksum, &content_stream, + tmp_to_buf = g_bytes_get_data (tmp_to, &tmp_to_len); + + if (!ostree_repo_load_file (repo, to_checksum, NULL, &content_finfo, &content_xattrs, cancellable, error)) goto out; @@ -754,9 +731,6 @@ process_one_rollsum (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_CLOSE); } - g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); - g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); - ret = TRUE; out: return ret; @@ -773,10 +747,11 @@ process_one_bsdiff (OstreeRepo *repo, { gboolean ret = FALSE; guint64 content_size; - g_autoptr(GInputStream) content_stream = NULL; g_autoptr(GFileInfo) content_finfo = NULL; g_autoptr(GVariant) content_xattrs = NULL; OstreeStaticDeltaPartBuilder *current_part = *current_part_val; + g_autoptr(GBytes) tmp_from = NULL; + g_autoptr(GBytes) tmp_to = NULL; const guint8 *tmp_to_buf; gsize tmp_to_len; const guint8 *tmp_from_buf; @@ -789,10 +764,17 @@ process_one_bsdiff (OstreeRepo *repo, *current_part_val = current_part = allocate_part (builder); } - tmp_to_buf = g_bytes_get_data (bsdiff_content->tmp_to, &tmp_to_len); - tmp_from_buf = g_bytes_get_data (bsdiff_content->tmp_from, &tmp_from_len); + if (!get_unpacked_unlinked_content (repo, bsdiff_content->from_checksum, &tmp_from, + cancellable, error)) + goto out; + if (!get_unpacked_unlinked_content (repo, to_checksum, &tmp_to, + cancellable, error)) + goto out; - if (!ostree_repo_load_file (repo, to_checksum, &content_stream, + tmp_to_buf = g_bytes_get_data (tmp_to, &tmp_to_len); + tmp_from_buf = g_bytes_get_data (tmp_from, &tmp_from_len); + + if (!ostree_repo_load_file (repo, to_checksum, NULL, &content_finfo, &content_xattrs, cancellable, error)) goto out; @@ -853,9 +835,6 @@ process_one_bsdiff (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_UNSET_READ_SOURCE); - g_clear_pointer (&bsdiff_content->tmp_from, g_bytes_unref); - g_clear_pointer (&bsdiff_content->tmp_to, g_bytes_unref); - ret = TRUE; out: return ret;