LockFileEx read/write upgrade/downgrade

I have the need to open a file, read-lock it, then attempt to get a write lock but keep the read lock if it fails.

This works great in POSIX using fcntl locking.

In Windows I can use LockFileEx to get file locks. I can get both read and write locks (shared and exclusive).

However, it seems that in Windows I must take the exclusive write lock first and then add the read lock. This is the opposite order of what I do on POSIX and it c开发者_开发百科auses problems for my abstraction layer. When I do it in that order in POSIX I lose the write lock by taking the read lock because fcntl replaces the existing lock instead of adding locks as Windows does.

I can hack it with #ifdefs to change the locking order at the call sites, but I am looking for good ideas to fix my abstraction code.

// This is the header file
struct LockFileImpl;
class LockFile {
    protected:
    boost::scoped_ptr<LockFileImpl> p;

    public:
    LockFile(const File &); 
    virtual ~LockFile();

    void unlock() const;
    void rd_lock() const;
    void wr_lock() const;
    bool rd_try() const;
    bool wr_try() const;
};

class LockFileRead : public LockFile{
    public:
    LockFileRead(const File &f) : LockFile(f)
    { rd_lock(); }
};

class LockFileWrite : public LockFile{
    public:
    LockFileWrite(const File &f) : LockFile(f)
    { wr_lock(); }
};

// This is the Win32 implementation file. There's a different one for POSIX.
struct LockFileImpl
{
    handle_t hFile;
    bool rd_locked;
    bool wr_locked;

    LockFileImpl(handle_t x) : hFile(x), rd_locked(false), wr_locked(false)
    {}
};

LockFile::LockFile(const File &f)
    : p( new LockFileImpl(f.handle()) )
{
}

LockFile::~LockFile()
{
    unlock();
}


void LockFile::unlock() const
{
    if(p->wr_locked) {
        throw_win32_err_if( UnlockFile(p->hFile, 0, 0, 1, 0) == 0 );
        p->wr_locked = false;
    }
    if(p->rd_locked) {
        throw_win32_err_if( UnlockFile(p->hFile, 0, 0, 1, 0) == 0 );
        p->rd_locked = false;
    }
}

void LockFile::rd_lock() const
{
    OVERLAPPED over = {0};
    over.Offset = 0;
    throw_win32_err_if( !LockFileEx(p->hFile, 0, 0, 1, 0, &over) );
    p->rd_locked = true;
    if(p->wr_locked) {
        throw_win32_err_if( UnlockFile(p->hFile, 0, 0, 1, 0) == 0 );
        p->wr_locked = false;
    }
}

void LockFile::wr_lock() const
{
    OVERLAPPED over = {0};
    over.Offset = 0;
    throw_win32_err_if( !LockFileEx(p->hFile, LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &over) );
    p->wr_locked = true;
}

bool LockFile::rd_try() const
{
    OVERLAPPED over = {0};
    over.Offset = 0;
    bool r = !!LockFileEx(p->hFile, LOCKFILE_FAIL_IMMEDIATELY, 0, 1, 0, &over);
    if(r) {
        p->rd_locked = true;
        if(p->wr_locked) {
            throw_win32_err_if( UnlockFile(p->hFile, 0, 0, 1, 0) == 0 );
            p->wr_locked = false;
        }
    }
    return r;
}

bool LockFile::wr_try() const
{
    OVERLAPPED over = {0};
    over.Offset = 0;
    bool r = !!LockFileEx(p->hFile, LOCKFILE_FAIL_IMMEDIATELY|LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &over);
    if(r) {
        p->wr_locked = true;
    }
    return r;
}

Why not use a pimpl approach? You're almost there, anyway. Create WinLockFileImpl and PosixLockFileImpl which both inherit an abstract LockFileImpl. Then, put one ifdef around the following code to determine which class is used at compile time. You must already have an ifdef that removes the windows code when compiling on other platforms anyway, right?

LockFile::LockFile(const File &f)
#ifdef POSIX
    : p( new PosixLockFileImpl(f.handle()) )
#else
    : p( new WinLockFileImpl(f.handle()) )
#endif

Oh, and you'll need to move your code into the implementation classes which changes LockFile to look more like this:

void LockFile::unlock() const
{
    p->unlock();
}

We have very limited locking requirements but the following code seems to work to mimic POSIX fcntl enough for our purposes. Note the hack to discriminate between read and write locks based on locked region size (this hack might work for you based on your example). The code below assumes files are less than 4GB.

// fcntl flock definitions
#define F_SETLK  8   // Non-Blocking set or clear a lock
#define F_SETLKW 9   // Blocking set or clear a lock
#define F_RDLCK  1   // read lock
#define F_WRLCK  2   // write lock
#define F_UNLCK  3   // remove lock
struct flock {
    short l_type;   // F_RDLCK, F_WRLCK, or F_UNLCK
    short l_whence; // flag to choose starting offset, must be SEEK_SET
    long  l_start;  // relative offset, in bytes, must be 0
    long  l_len;    // length, in bytes; 0 means lock to EOF, must be 0
    short l_pid;    // unused (returned with the unsupported F_GETLK)
    short l_xxx;    // reserved for future use
};

// only works for (SEEK_SET, start=0, len=0) file locking.
__inline int fcntl(int fd, int cmd, ...)
{
    va_list a;
    va_start(a, cmd);
    switch(cmd)
    {
    case F_SETLK:
        {
            struct flock *l = va_arg(a, struct flock*);
            switch(l->l_type)
            {
            case F_RDLCK:
                {
                    OVERLAPPED o = { 0 };
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    if (!LockFileEx(h, LOCKFILE_FAIL_IMMEDIATELY, 0, 0, 1, &o)) // read lock
                    {
                        unsigned long x = GetLastError();
                        _set_errno(GetLastError() == ERROR_LOCK_VIOLATION ? EAGAIN : EBADF);
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 1, 1); // write lock
                }
                break;
            case F_WRLCK:
                {
                    OVERLAPPED o = { 0 };
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    if (!LockFileEx(h, LOCKFILE_FAIL_IMMEDIATELY|LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 1, &o)) // write lock
                    {
                        unsigned long x = GetLastError();
                        _set_errno(GetLastError() == ERROR_LOCK_VIOLATION ? EAGAIN : EBADF);
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 0, 1); // read lock
                }
                break;
            case F_UNLCK:
                {
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 0, 1); // read lock
                    UnlockFile(h, 0, 0, 1, 1); // write lock
                }
                break;
            default:
                _set_errno(ENOTSUP);
                return -1;
            }
        }
        break;
    case F_SETLKW:
        {
            struct flock *l = va_arg(a, struct flock*);
            switch(l->l_type)
            {
            case F_RDLCK:
                {
                    OVERLAPPED o = { 0 };
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    if(!LockFileEx(h, 0, 0, 0, 1, &o)) // read lock
                    {
                        unsigned long x = GetLastError();
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 1, 1); // write lock
                }
                break;
            case F_WRLCK:
                {
                    OVERLAPPED o = { 0 };
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    if (!LockFileEx(h, LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 1, &o)) // write lock
                    {
                        unsigned long x = GetLastError();
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 0, 1); // read lock
                }
                break;
            case F_UNLCK:
                {
                    flock *l = va_arg(a, flock*);
                    HANDLE h = (HANDLE)_get_osfhandle(fd);
                    if (l->l_whence != SEEK_SET || l->l_start != 0 || l->l_len != 0)
                    {
                        _set_errno(ENOTSUP);
                        return -1;
                    }
                    UnlockFile(h, 0, 0, 0, 1); // read lock
                    UnlockFile(h, 0, 0, 1, 1); // write lock
                }
                break;
            default:
                _set_errno(ENOTSUP);
                return -1;
            }
        }
        break;
    default:
        _set_errno(ENOTSUP);
        return -1;
    }

    return 0;
}

The main problem with fcntl locking to FileLock locking is, as you noted, that little caveat that (from the docs):

If the same range is locked with an exclusive and a shared lock, two unlock operations are necessary to unlock the region; the first unlock operation unlocks the exclusive lock, the second unlock operation unlocks the shared lock.

This means you cannot go from just a shared lock to just an exclusive lock of the same region without first releasing the lock completely on that region. Your method of using flags comes close and I think if you added another flag that says i_have_both_locks_but_only_really_want_the_exclusive_lock then your solution could work. We didn't have the luxury of writing our own interface (we were stuck with fcntl). But, luckily, the code using fcntl only just ever wanted to lock the entire file and the files were small. An alternative solution would have been to put a std::map in the fcntl call to keep track of the fcntl locks vs the FileLock locks owned.