Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for opening files with FILE_SHARE_DELETE on Windows #59449

Open
sbt mannequin opened this issue Jul 3, 2012 · 12 comments
Open

Support for opening files with FILE_SHARE_DELETE on Windows #59449

sbt mannequin opened this issue Jul 3, 2012 · 12 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-windows topic-IO type-feature A feature request or enhancement

Comments

@sbt
Copy link
Mannequin

sbt mannequin commented Jul 3, 2012

BPO 15244
Nosy @loewis, @pfmoore, @ncoghlan, @pitrou, @tjguk, @bitdancer, @dabrahams, @zware, @eryksun, @zooba
Files
  • share.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2012-07-03.19:14:54.411>
    labels = ['3.8', '3.9', 'expert-IO', 'type-feature', '3.10', 'OS-windows']
    title = 'Support for opening files with FILE_SHARE_DELETE on Windows'
    updated_at = <Date 2021-03-12.23:13:50.557>
    user = 'https://bugs.python.org/sbt'

    bugs.python.org fields:

    activity = <Date 2021-03-12.23:13:50.557>
    actor = 'eryksun'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Windows', 'IO']
    creation = <Date 2012-07-03.19:14:54.411>
    creator = 'sbt'
    dependencies = []
    files = ['26249']
    hgrepos = []
    issue_num = 15244
    keywords = ['patch']
    message_count = 12.0
    messages = ['164616', '184087', '184089', '184129', '184158', '184162', '184164', '228368', '228380', '228437', '239163', '388564']
    nosy_count = 12.0
    nosy_names = ['loewis', 'paul.moore', 'ncoghlan', 'pitrou', 'tim.golden', 'r.david.murray', 'dabrahams', 'sbt', 'piotr.dobrogost', 'zach.ware', 'eryksun', 'steve.dower']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15244'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @sbt
    Copy link
    Mannequin Author

    sbt mannequin commented Jul 3, 2012

    On Unix, files (unless specifically locked) can be renamed and deleted while file descriptors for the file remain open. The file descriptors remain valid even after deletion of the file.

    On Windows this is not possible for files opened using io.open() or os.open(). However, by using the FILE_SHARE_DELETE flag in CreateFile() one can get Unix-like behaviour.

    Unfortunately, FILE_SHARE_DELETE is only available through the Win32 API, not through the CRT. Also, Issue bpo-14243 concerns the fact that on Windows temporary files cannot be reopened unless one uses the FILE_SHARE_DELETE flag. One can only reopen a file by using a share mode that is at least as permissive as the share mode for all the currently open handles.

    The attached patch adds a module "share" (bad name?) with functions share.open() and share.os_open() which act as substitutes for io.open() and os.open(). These by default use FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE as the share mode. (io.open() and os.open() use FILE_SHARE_READ | FILE_SHARE_WRITE instead.)

    To run the full regression test suite with builtins.open(), io.open() and os.open() monkey patched to use these replacements you can do

    python -m test.test_share --regrtest
    

    Nothing seems to break.

    @sbt sbt mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jul 3, 2012
    @piotrdobrogost
    Copy link
    Mannequin

    piotrdobrogost mannequin commented Mar 13, 2013

    Having the same semantics on both Unix and Windows with regard to validity of file handle after a file was deleted would be a very nice to have. How could we progress this?

    I'm adding Martin and Antoine to cc list.

    @sbt
    Copy link
    Mannequin Author

    sbt mannequin commented Mar 13, 2013

    Actually, it is not quite the same semantics as Unix.

    After you delete the the file you cannot create a file of the same name or delete the directory which contains it until the handle has been closed.

    However, one can work around that by moving the file somewhere else (like the root directory) before deleting it.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 14, 2013

    I don't understand whether you are proposing to include the patch into Python as-is; if so, I'm -1 on it for two formal reasons: a) the standard library shouldn't monkey patch itself, and b) OS interfacing should be implemented in C.

    That said, having maximum sharing when opnening files sounds fine to me.

    @piotrdobrogost
    Copy link
    Mannequin

    piotrdobrogost mannequin commented Mar 14, 2013

    I don't understand whether you are proposing to include the patch into Python as-is;

    I think Richard is well aware of the constraints you specify and current patch was meant as a proof of concept; to show that all tests pass with such a change. Of course that's only my belief and we shall see what Richard has to say.

    That said, having maximum sharing when opnening files sounds fine to me.

    Good to hear. However I started to wonder if we are ready for all consequences of this. For example taking into account what Richard noted in http://bugs.python.org/issue14243, specifically:

    Unfortunately using O_TEMPORARY is the only way allowed by msvcrt to
    get FILE_SHARE_DELETE, even though it also has the orthogonal effect
    of unlinking the file when all handles are closed.

    forces programs which would like to open a file being opened at the same time by Python code (by means of built-in open() or os.open() with default arguments) to either use O_TEMPORARY when using msvcrt or to go low level and use CreateFile() Win32 API function with FILE_SHARE_DELETE flag. Are we ok with it?

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 14, 2013

    Am 14.03.13 03:31, schrieb Piotr Dobrogost:

    forces programs which would like to open a file being opened at the
    same time by Python code (by means of built-in open() or os.open()
    with default arguments) to either use O_TEMPORARY when using msvcrt
    or to go low level and use CreateFile() Win32 API function with
    FILE_SHARE_DELETE flag. Are we ok with it?

    That's why I was asking for an actual patch. The proposed change may
    well not be implementable. If os.open continues to create CRT handles,
    a way needs to be found to get a CRT handle that as the
    FILE_SHARE_DELETE bit set.

    An alternative approach could be that os.open stops creating CRT
    handles, and directly uses OS handles. The problem with that is that
    stdin/stdout/stderr would stop being 0/1/2, which is not acceptable.
    An alternative solution to that could be that we introduce a notion
    of "python io handles", parallel, but indepedendent from CRT handles.
    And so on.

    @sbt
    Copy link
    Mannequin Author

    sbt mannequin commented Mar 14, 2013

    On 14/03/2013 1:00pm, Martin v. Löwis wrote:

    That's why I was asking for an actual patch. The proposed change may
    well not be implementable. If os.open continues to create CRT handles,
    a way needs to be found to get a CRT handle that as the
    FILE_SHARE_DELETE bit set.

    The patch *does* create CRT fds from win32 handles by using
    msvcrt.open_osfhandle().

    One other issue is that I do not know of a way to determine the current
    umask without temporarily changing it, causing a thread-race.

    In the end I am not sure it is worth the hassle. (But maybe it would be
    a good idea to add test.support.open() using FILE_SHARE_DELETE and
    test.support.unlink() to make the testsuite more resilient to
    "Permission Denied" errors.)

    An alternative approach could be that os.open stops creating CRT
    handles, and directly uses OS handles. The problem with that is that
    stdin/stdout/stderr would stop being 0/1/2, which is not acceptable.
    An alternative solution to that could be that we introduce a notion
    of "python io handles", parallel, but indepedendent from CRT handles.
    And so on.

    http://bugs.python.org/issue12939 has C implementations of _io.WinFileIO
    and _io.openhandle() which are equivalents for _io.FileIO and os.open()
    which use "native" Windows handles.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Oct 3, 2014

    How do we take this forward, also considering bpo-12939, bpo-21859 and bpo-14243 and possibly others?

    @zooba
    Copy link
    Member

    zooba commented Oct 3, 2014

    As much as I like the idea of using OS handles everywhere, the compatibility issues are probably too significant, and you can't mix CRT methods with OS methods because the CRT does its own buffering. Of course, you can open a file with the Win32 API and immediately call open_osfhandle() safely enough.

    It seems this should be solvable with the opener argument of open(). Was that discussed and I missed it? Would the ability to write "open(f, opener=with_same_sharing_as(f))" be okay/useful/relevant for all platforms?

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Oct 4, 2014

    "opener" was new in 3.3, so I suspect it didn't come up because *we're* still not entirely accustomed to having it available yet :)

    I agree providing a suitable opener could be a good way to go.

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 24, 2015

    you can't mix CRT methods with OS methods because the CRT does its
    own buffering

    Python 3's io uses the CRT's low I/O (ioinfo struct) in binary mode. It appears that the buffers pipech, pipech2, and dbcsbuffer are only used in text mode. So it should be generally OK to call get_osfhandle and use Win32 directly, if that's ever required.

    Of course, as this patch demonstrates, an io opener (3.3+) can call open_osfhandle immediately after CreateFile, and nothing else changes. This could be added to io / _pyio in 3.5. Initially it could simply be a pure-Python function that uses _winapi and msvcrt. 3.6 could get a C implementation in _iomodule.c.

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 12, 2021

    Deleting a file in Windows 10 has been updated to try a POSIX-style delete. For a POSIX delete, the filesystem unlinks the file even it's open, whereas a classic delete only unlinks a 'deleted' file when it's closed. The filesystem has to support it. NTFS does, which is the most important because the system drive is NTFS. This makes supporting FILE_SHARE_DELETE a more attractive option.

    That said, there are sill significant limits. Memory-mapped files can't be deleted (renamed, yes, but not removed). Also, most programs don't share delete access on their opens, which means they can't open a file that's currently open with delete access (e.g. NamedTemporaryFile), and their open files can't be deleted.

    @eryksun eryksun added OS-windows topic-IO 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes and removed stdlib Python modules in the Lib dir labels Mar 12, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-windows topic-IO type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants