[arrow] branch master updated: ARROW-3936: [C++] Add _O_NOINHERIT to the file open flags on Windows

[ap...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 0afb975  ARROW-3936: [C++] Add _O_NOINHERIT to the file open flags on Windows
0afb975 is described below

commit 0afb975fb55aeb04bc1034678fd3dca33b7c7c0e
Author: philjdf <gi...@pfelton.co.uk>
AuthorDate: Wed Dec 5 11:59:21 2018 +0100

    ARROW-3936: [C++] Add _O_NOINHERIT to the file open flags on Windows
    
    Unlike Linux, Windows doesn't let you delete files that are currently opened by another process. So if you create a child process while a Parquet file is open, with the current code the file handle is inherited to the child process, and the parent process can't then delete the file after closing it without the child process terminating first.
    
    By default, Win32 file handles are not inheritable (likely because of the aforementioned problems). Except for _wsopen_s, which tries to maintain POSIX compatibility.
    
    This is a serious problem for us.
    
    We would argue that specifying _O_NOINHERIT by default in the _MSC_VER path is a sensible approach and would likely be the correct behaviour as it matches the main Win32 API.
    
    However, it could be that some developers rely on the current inheritable behaviour. In which case, the Arrow public API should take a boolean argument on whether the created file descriptor should be inheritable. But this would break API backward compatibility (unless a new overloaded method is introduced).
    
    Is forking and inheriting Arrow internal file descriptor something that Arrow actually means to support?
    
    What do we think of the proposed fix?
    
    Author: philjdf <gi...@pfelton.co.uk>
    Author: Phil Felton <39...@users.noreply.github.com>
    
    Closes #3085 from philjdf/InheritableHandlesFix and squashes the following commits:
    
    6b4be85d <Phil Felton> Fix linting issue
    a9dda7fb <philjdf> Do not share handles on Windows
---
 cpp/src/arrow/util/io-util.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/util/io-util.cc b/cpp/src/arrow/util/io-util.cc
index 8db5db4..74ad806 100644
--- a/cpp/src/arrow/util/io-util.cc
+++ b/cpp/src/arrow/util/io-util.cc
@@ -146,8 +146,8 @@ Status FileNameFromString(const std::string& file_name, PlatformFilename* out) {
 Status FileOpenReadable(const PlatformFilename& file_name, int* fd) {
   int ret, errno_actual;
 #if defined(_MSC_VER)
-  errno_actual = _wsopen_s(fd, file_name.wstring().c_str(), _O_RDONLY | _O_BINARY,
-                           _SH_DENYNO, _S_IREAD);
+  errno_actual = _wsopen_s(fd, file_name.wstring().c_str(),
+                           _O_RDONLY | _O_BINARY | _O_NOINHERIT, _SH_DENYNO, _S_IREAD);
   ret = *fd;
 #else
   ret = *fd = open(file_name.c_str(), O_RDONLY | O_BINARY);
@@ -162,7 +162,7 @@ Status FileOpenWritable(const PlatformFilename& file_name, bool write_only, bool
   int ret, errno_actual;
 
 #if defined(_MSC_VER)
-  int oflag = _O_CREAT | _O_BINARY;
+  int oflag = _O_CREAT | _O_BINARY | _O_NOINHERIT;
   int pmode = _S_IWRITE;
   if (!write_only) {
     pmode |= _S_IREAD;