Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#4363 closed bug (fixed)

openFile sharing permissions are inconsistent across platforms

Reported by: jystic Owned by:
Priority: high Milestone: 7.6.1
Component: libraries/base Version: 6.12.3
Keywords: Cc: pho@…
Operating System: Windows Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

System.IO.openFile seems to have inconsistent behaviour across platforms regarding file sharing permissions.

Given this program:

{-# LANGUAGE CPP #-}

import System.IO
import System.Process

main = do
    h <- openFile "file.txt" WriteMode
    hPutStrLn h "Success! I can see the file's contents."
    hFlush h
#ifdef mingw32_HOST_OS
    system "type file.txt"
#else
    system "cat file.txt"
#endif
    hClose h

Running under Ubuntu 10.04 / GHC 6.12.1, I get:

$ runghc openFile.hs
Success! I can see the file's contents.

Running under Windows 7 / GHC 6.12.3, I get:

> runghc openFile.hs
The process cannot access the file because it is being used by another process.

In my opinion the behaviour exhibited by Linux is preferable because it allows for log files to be written by long running processes. These log files can then be inspected externally while the Haskell process is still running.

The Snap Framework (snapframework.com) does this and it works great on Linux / Mac OS, but on Windows the log files cannot be viewed until after the server is shut down.

Attachments (2)

4363.patch (11.6 KB) - added by pcapriotti 3 years ago.
4363-base.patch (9.2 KB) - added by pcapriotti 3 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 years ago by malcolm.wallace@…

This is just the way Windows works. It is unlikely ghc can do anything to work around that Windows feature/bug. However, you (the programmer) can explicitly hClose the file, print it, and then reopen it in AppendMode if you want to. This is not a general solution, but might be OK in certain application-specific circumstances.

comment:2 Changed 5 years ago by guest

Malcolm,

Could you elaborate on what you mean by "that's just the way Windows works"? I haven't looked at the file io implementation for GHC, but Windows is quite flexible here, in Windows msvcrt (stdio) is implemented in terms of CreateFile iirc, which has different sharing options. So it should be possible to match the behavior under Ubuntu.

I did a quick test just in C on Vista (where the Haskell example reproduces the undesirable result under 6.10.2 also):

#include <stdio.h>
#include <stdlib.h>
int main(void)
{
  FILE* foo;
  FILE* bar;
  char buf[32];

  foo = fopen("log.txt", "w");
  if(!foo)
    {
      puts("error opening log.txt for writing");
      return EXIT_FAILURE;
    }
  puts("Writing some text to log.txt\n");
  fprintf(foo, "1,2,3,6,11,23,47,106,235\n");
  fflush(foo);

  bar = fopen("log.txt", "r");
  if(!bar)
    {
      puts("error opening log.txt for reading");
      return EXIT_FAILURE;
    }

  if(!fgets(buf, 31, bar))
    {
      puts("error reading from stream");
      return EXIT_FAILURE;
    } else { printf("read: (%s) from file while it was open for writing.\n", buf); }

    fclose(foo);
    fclose(bar);
    
 return EXIT_SUCCESS;
}

 

Which successfully reads the file that was open for writing:

Output when compiled with gcc from mingw 3.4.5:

Writing some text to log.txt

read: (1,2,3,6,11,23,47,106,235
) from file while it was open for writing.

comment:3 Changed 5 years ago by jystic

I'm not 100% sure, but it looks like openFile ends up calling __hscore_open from libraries\base\include\HsBase.h:

#ifdef __MINGW32__
INLINE int __hscore_open(wchar_t *file, int how, mode_t mode) {
	if ((how & O_WRONLY) || (how & O_RDWR) || (how & O_APPEND))
	  return _wsopen(file,how | _O_NOINHERIT,_SH_DENYRW,mode);
          // _O_NOINHERIT: see #2650
	else
	  return _wsopen(file,how | _O_NOINHERIT,_SH_DENYWR,mode);
          // _O_NOINHERIT: see #2650
}
#else
INLINE int __hscore_open(char *file, int how, mode_t mode) {
	return open(file,how,mode);
}
#endif

The documentation for _wsopen suggests that there are four options that can be passed to it to change the sharing behaviour:

_SH_DENYRW Denies read and write access to a file.

_SH_DENYWR Denies write access to a file.

_SH_DENYRD Denies read access to a file.

_SH_DENYNO Permits read and write access.

We pass _SH_DENYRW (deny reads, deny writes) in the case that we are writing to the file and _SH_DENYWR (deny writes, allow reads) in the case that we are reading from the file but not writing to it.

If we were to always use _SH_DENYWR then I think it would give the desired behaviour.

comment:4 Changed 5 years ago by simonmar

The Haskell standard requires us to implement single-writer/multiple-reader locking, which is what the above code is doing. Unfortunately the locking extends outside the process, which is unfortunate (but is in the spirit of the Haskell locking requirement).

One reason for the locking is so that you can't open a file for writing that you also have open for lazy I/O. However, there are other ways to achieve unexpected results with lazy I/O. My personal view is that this locking has caused more trouble and surprising behaviour than it saves, and we should just default to whatever locking semantics the OS provides. I'd be interested to hear what others think about this. Maybe we could do a Haskell 2011/2012 proposal to get it changed, and in the meantime we could change it in GHC too.

comment:5 follow-up: Changed 5 years ago by NeilMitchell

I've been bitten by this in practice several times, as has Max Bolingbroke. We have things where Haskell writes out a file which we than cat in to the file we really wanted to write to avoid this problem. It's even more annoying that it's done to satisfy the standard, but that on Linux you don't satisfy the standard and it's far more convenient. Windows is perfectly capable of doing what Linux does - I'd love it to be changed.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 5 years ago by simonmar

Replying to NeilMitchell:

I've been bitten by this in practice several times, as has Max Bolingbroke. We have things where Haskell writes out a file which we than cat in to the file we really wanted to write to avoid this problem. It's even more annoying that it's done to satisfy the standard, but that on Linux you don't satisfy the standard and it's far more convenient. Windows is perfectly capable of doing what Linux does - I'd love it to be changed.

I understand the inconvenience, but just wanted to point out that we do comply with the standard on Linux. The difference is that on Linux we implement the locking ourselves, locally to the Haskell process. We could have used flock() on Linux, and conversely, we could probably implement our own in-process locking on Windows (not that I think we should do this).

Anyway, the current situation is both inconsistent and inconvenient, and I agree we should do something about it.

comment:7 Changed 5 years ago by igloo

  • Milestone set to 7.2.1
  • Priority changed from normal to high

comment:8 Changed 4 years ago by jystic

What needs to be done to help move this item along? If I submit a patch for HsBase.h would that help?

comment:9 Changed 4 years ago by igloo

I think the next step should be a Haskell' proposal to remove the locking from the report (including a clarification that a no-locking implementation is possible on Windows).

Some people might argue that we should have an implementation first, though.

comment:10 Changed 4 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1

I changed my mind, having realised that this is really more of a library change rather than a language change.

I made a proposal; thread "Proposal: Stop enforcing single-writer-multi-reader file access" starts here: http://www.haskell.org/pipermail/libraries/2011-October/016978.html and continues into November.

However, it fizzled after Duncan counter-proposed making consistent locking the default, and providing a way to opt-out of locking. There was no clear conclusion as to which solution was preferred.

comment:11 in reply to: ↑ 6 Changed 3 years ago by jystic

I've read through the mailing list archives and I can understand where Duncan is coming from. The lazy IO problem certainly complicates matters.

Would it be feasible to implementing in-process locking on Windows?

It seems to me like that would be a solution which is good enough to solve this specific problem. Sure it would be great to offer a more advanced file API in the long term, but it could be a while before the community comes to a consensus on the right design.

I see that the POSIX file locking code uses the device and inode as a unique identifier for files. Unfortunately, Windows doesn't return sensible information for st_ino when using fstat. You can however get a similar thing using GetFileInformationByHandle.

Here's an example I just tested out using gcc on Windows.

#include <stdio.h>
#include <share.h>
#include <fcntl.h>
#include <windows.h>

int main()
{
    const wchar_t *file = L"file.txt";

    int fd;
    HANDLE h;

    BY_HANDLE_FILE_INFORMATION info;
    long long inode;
    int device; 

    /* assuming file already exists */
    fd = _wsopen(file, O_RDWR | _O_NOINHERIT, _SH_DENYWR, 0);

    /* convert file descriptor to windows handle */
    h = (HANDLE)_get_osfhandle(fd);

    if (GetFileInformationByHandle(h, &info))
    {
        device = info.dwVolumeSerialNumber;
        inode  = info.nFileIndexLow | ((long long)info.nFileIndexHigh << 32);

        printf("device: %x\n", device);
        printf("inode: %I64x\n", inode);
    }

    return 0;
}

comment:12 Changed 3 years ago by jystic

On the topic of sharing permissions being inconsistent across platforms, there is another scenario which I have noticed that probably needs attention as well. Perhaps it needs its own ticket, but it could be part of this clean up.

When we open a file on POSIX systems, it is legal to delete or rename the file even though it is open. On Windows however, we are locking the file so that it may not be deleted or renamed.

This is purely because we are using the POSIX style APIs (i.e. _wsopen) for opening files. These APIs do not have flags for specifying all the possible sharing permissions and unfortunately Windows defaults to preventing open files from being deleted or renamed.

This program on POSIX will allow the test file to be deleted while the program is running (standard behaviour):


#include <stdio.h>
#include <fcntl.h>
#include <string.h>
#include <unistd.h>

int main()
{
    const char *file = "./test.txt";

    int fd;
    int i = 0;
    char buf[16];

    fd = open(file, O_WRONLY | O_CREAT, 0644);

    while (1)
    {
        sprintf(buf, "%d\n", i++);
        write(fd, buf, strlen(buf));
        sleep(1);
    }

    return 0;
}


The following program achieves the same behaviour on Windows by using the FILE_SHARE_DELETE flag:

#include <stdio.h>
#include <fcntl.h>
#include <windows.h>

int main()
{
    const wchar_t *file = L"test.txt";

    int fd;
    HANDLE h;
    int i = 0;
    char buf[16];

    h = CreateFileW(file,
            GENERIC_WRITE,
            FILE_SHARE_READ | FILE_SHARE_DELETE,
            NULL,
            CREATE_NEW,
            FILE_ATTRIBUTE_NORMAL,
            NULL);

    fd = _open_osfhandle((intptr_t)h, 0);

    while (1)
    {
        sprintf(buf, "%d\n", i++);
        write(fd, buf, strlen(buf));
        Sleep(1000);
    }

    return 0;
}

Changed 3 years ago by pcapriotti

Changed 3 years ago by pcapriotti

comment:13 Changed 3 years ago by pcapriotti

  • difficulty set to Unknown
  • Status changed from new to patch

I attached patches to enable in-process file locking on Windows (thanks jystic for the example code).

This doesn't change the behavior significantly, only makes it consistent with POSIX platforms. Does it need a new library proposal?

comment:14 Changed 3 years ago by p.capriotti@…

commit 9fb12e142b80ce399285b41e8fad4f862bb6a776

Author: Paolo Capriotti <[email protected]>
Date:   Tue May 8 14:05:28 2012 +0100

    Enable FileLock for win32 (#4363)

 includes/rts/FileLock.h |    6 +-
 rts/FileLock.c          |  144 ++++++++++++++++++++++++++++++++++++++++++++++
 rts/FileLock.h          |   15 +++++
 rts/Linker.c            |    4 +-
 rts/RtsStartup.c        |    6 +--
 rts/posix/FileLock.c    |  145 -----------------------------------------------
 rts/posix/FileLock.h    |   15 -----
 7 files changed, 164 insertions(+), 171 deletions(-)

comment:15 Changed 3 years ago by pcapriotti

  • Status changed from patch to merge

comment:16 Changed 3 years ago by pcapriotti

  • Resolution set to fixed
  • Status changed from merge to closed

comment:17 Changed 3 years ago by PHO

  • Cc pho@… added
Note: See TracTickets for help on using tickets.