[21/31] mesos git commit: Windows: Made `net::download()` use CRT file descriptor explicitly.

[an...@apache.org]

Windows: Made `net::download()` use CRT file descriptor explicitly.

This is an edge case where a third-party library (libcurl) requires a
CRT integer file descriptor. Thus we explicitly allocate one via
`crt()`, which requires that we also manually close it via `_close()`,
not `os::close()`.

Review: https://reviews.apache.org/r/66433


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b061fb14
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b061fb14
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b061fb14

Branch: refs/heads/master
Commit: b061fb14638f306491d16cc79442ca5bc46bbde6
Parents: d0b055b
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Tue Apr 3 22:33:39 2018 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue May 1 18:36:04 2018 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/net.hpp | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b061fb14/3rdparty/stout/include/stout/net.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/net.hpp b/3rdparty/stout/include/stout/net.hpp
index 52fa09b..10666ed 100644
--- a/3rdparty/stout/include/stout/net.hpp
+++ b/3rdparty/stout/include/stout/net.hpp
@@ -167,19 +167,32 @@ inline Try<int> download(
   curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, nullptr);
   curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, true);
 
-  // We don't bother introducing a `os::fdopen` since this is the only place
-  // we use `fdopen` in the entire codebase as of writing this comment.
+  // We don't bother introducing a `os::fdopen()` since this is the
+  // only place we use `fdopen()` in the entire codebase as of writing
+  // this comment.
 #ifdef __WINDOWS__
+  // This explicitly allocates a CRT integer file descriptor, which
+  // when closed, also closes the underlying handle, so we do not call
+  // `CloseHandle()` (or `os::close()`).
+  const int crt = fd->crt();
   // We open in "binary" mode on Windows to avoid line-ending translation.
-  FILE* file = ::_fdopen(fd->crt(), "wb");
+  FILE* file = ::_fdopen(crt, "wb");
+  if (file == nullptr) {
+    curl_easy_cleanup(curl);
+    // NOTE: This is not `os::close()` because we allocated a CRT int
+    // fd earlier.
+    ::_close(crt);
+    return ErrnoError("Failed to open file handle of '" + path + "'");
+  }
 #else
   FILE* file = ::fdopen(fd.get(), "w");
-#endif
   if (file == nullptr) {
     curl_easy_cleanup(curl);
     os::close(fd.get());
     return ErrnoError("Failed to open file handle of '" + path + "'");
   }
+#endif // __WINDOWS__
+
   curl_easy_setopt(curl, CURLOPT_WRITEDATA, file);
 
   if (stall_timeout.isSome()) {
@@ -195,7 +208,9 @@ inline Try<int> download(
   CURLcode curlErrorCode = curl_easy_perform(curl);
   if (curlErrorCode != 0) {
     curl_easy_cleanup(curl);
-    fclose(file);
+    // NOTE: `fclose()` also closes the associated file descriptor, so
+    // we do not call `close()`.
+    ::fclose(file);
     return Error(curl_easy_strerror(curlErrorCode));
   }
 
@@ -203,7 +218,7 @@ inline Try<int> download(
   curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &code);
   curl_easy_cleanup(curl);
 
-  if (fclose(file) != 0) {
+  if (::fclose(file) != 0) {
     return ErrnoError("Failed to close file handle of '" + path + "'");
   }