From 59adfebcacd3521a09b77cf18c9ef9f9ee02aeff Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Thu, 14 May 2020 20:25:39 -0400 Subject: [PATCH 1/2] Throw exception instead of silently failing if zip save/close fails I saw a case before on my machine where closing the zip would fail to write out changes, but it would fail silently, so it wasn't obvious there was a problem there. See more details here: https://xamarinhq.slack.com/archives/C03CEGRUW/p1588784048377800 This PR makes those errors explicit, throwing an exception on Close failures, so that the user will see there's a problem, that the ZIP/APK didn't actually get updated. I'm honestly a little nervous about this change - it seems good conceptually but might there be cases in practice where it's better to fail silently? I'm not sure, but I wanted to create a PR to get feedback from you all on that. As for my machine, it was a failure to create temp file error (see Slack). The problem went away after I rebooted and I wasn't able to reproduce it again. Probably it was some file in use issue of some kind. This may or may not be a problem customers see in practice (of course, since it fails silently, we have less visibility into whether it is a problem in practice - making the error explicit would help with that at least). --- ZipArchive.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ZipArchive.cs b/ZipArchive.cs index a076d3a5..0113afe6 100644 --- a/ZipArchive.cs +++ b/ZipArchive.cs @@ -840,12 +840,17 @@ public void Close () if (archive == IntPtr.Zero) return; - Native.zip_close (archive); - foreach (var s in sources) { - s.Dispose (); + try { + if (Native.zip_close (archive) < 0) + throw GetErrorException (); + } + finally { + foreach (var s in sources) { + s.Dispose (); + } + sources.Clear (); + archive = IntPtr.Zero; } - sources.Clear (); - archive = IntPtr.Zero; } /// From 5a61c24317367562e4dc6f6096a6ed493ed1f8f5 Mon Sep 17 00:00:00 2001 From: Bret Johnson Date: Fri, 15 May 2020 12:30:30 -0400 Subject: [PATCH 2/2] Fix finally formatting --- ZipArchive.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ZipArchive.cs b/ZipArchive.cs index 0113afe6..38e1f8ee 100644 --- a/ZipArchive.cs +++ b/ZipArchive.cs @@ -843,8 +843,7 @@ public void Close () try { if (Native.zip_close (archive) < 0) throw GetErrorException (); - } - finally { + } finally { foreach (var s in sources) { s.Dispose (); }