From b09d15d50330f050f158b41e5b7a3ce572c09d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Sat, 17 Mar 2018 16:31:54 +0100 Subject: [PATCH] Fix dangling handles resulting from actively closing a file. --- source/eventcore/driver.d | 8 ++++++ source/eventcore/drivers/threadedfile.d | 35 ++++++++++++++++++------- source/eventcore/drivers/winapi/files.d | 24 ++++++++++++----- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/source/eventcore/driver.d b/source/eventcore/driver.d index 6323890..ddd196b 100644 --- a/source/eventcore/driver.d +++ b/source/eventcore/driver.d @@ -371,6 +371,14 @@ interface EventDriverFiles { @safe: /*@nogc:*/ nothrow: FileFD open(string path, FileOpenMode mode); FileFD adopt(int system_file_handle); + + /** Disallows any reads/writes and removes any exclusive locks. + + Note that this function may not actually close the file handle. The + handle is only guaranteed to be closed one the reference count drops + to zero. However, the remaining effects of calling this function will + be similar to actually closing the file. + */ void close(FileFD file); ulong getSize(FileFD file); diff --git a/source/eventcore/drivers/threadedfile.d b/source/eventcore/drivers/threadedfile.d index 7f0a3f1..e87c99f 100644 --- a/source/eventcore/drivers/threadedfile.d +++ b/source/eventcore/drivers/threadedfile.d @@ -101,6 +101,7 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil static struct FileInfo { IOInfo read; IOInfo write; + bool open = true; int refCount; DataInitializer userDataDestructor; @@ -175,7 +176,11 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil void close(FileFD file) { - () @trusted { .close(cast(int)file); } (); + // NOTE: The file descriptor itself must stay open until the reference + // count drops to zero, or this would result in dangling handles. + // In case of an exclusive file lock, the lock should be lifted + // here. + m_files[file].open = false; } ulong getSize(FileFD file) @@ -193,11 +198,17 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil final override void write(FileFD file, ulong offset, const(ubyte)[] buffer, IOMode, FileIOCallback on_write_finish) { //assert(this.writable); - auto f = &m_files[file].write; - if (!safeCAS(f.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated)) + auto f = () @trusted { return &m_files[file]; } (); + + if (!f.open) { + on_write_finish(file, IOStatus.disconnected, 0); + return; + } + + if (!safeCAS(f.write.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated)) assert(false, "Concurrent file writes are not allowed."); - assert(f.callback is null, "Concurrent file writes are not allowed."); - f.callback = on_write_finish; + assert(f.write.callback is null, "Concurrent file writes are not allowed."); + f.write.callback = on_write_finish; m_activeWrites.insert(file); threadSetup(); log("start write task"); @@ -224,11 +235,17 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil final override void read(FileFD file, ulong offset, ubyte[] buffer, IOMode, FileIOCallback on_read_finish) { - auto f = &m_files[file].read; - if (!safeCAS(f.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated)) + auto f = () @trusted { return &m_files[file]; } (); + + if (!f.open) { + on_read_finish(file, IOStatus.disconnected, 0); + return; + } + + if (!safeCAS(f.read.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated)) assert(false, "Concurrent file reads are not allowed."); - assert(f.callback is null, "Concurrent file reads are not allowed."); - f.callback = on_read_finish; + assert(f.read.callback is null, "Concurrent file reads are not allowed."); + f.read.callback = on_read_finish; m_activeReads.insert(file); threadSetup(); log("start read task"); diff --git a/source/eventcore/drivers/winapi/files.d b/source/eventcore/drivers/winapi/files.d index 035a5f6..03c44ef 100644 --- a/source/eventcore/drivers/winapi/files.d +++ b/source/eventcore/drivers/winapi/files.d @@ -27,7 +27,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles { auto access = mode == FileOpenMode.readWrite || mode == FileOpenMode.createTrunc ? (GENERIC_WRITE | GENERIC_READ) : mode == FileOpenMode.append ? GENERIC_WRITE : GENERIC_READ; - auto shareMode = mode == FileOpenMode.read ? FILE_SHARE_READ : 0; + auto shareMode = FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE; auto creation = mode == FileOpenMode.createTrunc ? CREATE_ALWAYS : mode == FileOpenMode.append? OPEN_ALWAYS : OPEN_EXISTING; auto handle = () @trusted { @@ -76,7 +76,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles { auto h = idToHandle(file); auto slot = () @trusted { return &m_core.m_handles[h].file(); } (); if (slot.read.overlapped.hEvent != INVALID_HANDLE_VALUE) { - CloseHandle(h); + slot.read.overlapped.hEvent = slot.write.overlapped.hEvent = INVALID_HANDLE_VALUE; } } @@ -92,13 +92,19 @@ final class WinAPIEventDriverFiles : EventDriverFiles { override void write(FileFD file, ulong offset, const(ubyte)[] buffer, IOMode mode, FileIOCallback on_write_finish) { + auto h = idToHandle(file); + auto slot = &m_core.m_handles[h].file.write; + + if (slot.overlapped.hEvent == INVALID_HANDLE_VALUE) { + on_write_finish(file, IOStatus.disconnected, 0); + return; + } + if (!buffer.length) { on_write_finish(file, IOStatus.ok, 0); return; } - auto h = idToHandle(file); - auto slot = &m_core.m_handles[h].file.write; slot.bytesTransferred = 0; slot.offset = offset; slot.buffer = buffer; @@ -110,13 +116,19 @@ final class WinAPIEventDriverFiles : EventDriverFiles { override void read(FileFD file, ulong offset, ubyte[] buffer, IOMode mode, FileIOCallback on_read_finish) { + auto h = idToHandle(file); + auto slot = &m_core.m_handles[h].file.read; + + if (slot.overlapped.hEvent == INVALID_HANDLE_VALUE) { + on_read_finish(file, IOStatus.disconnected, 0); + return; + } + if (!buffer.length) { on_read_finish(file, IOStatus.ok, 0); return; } - auto h = idToHandle(file); - auto slot = &m_core.m_handles[h].file.read; slot.bytesTransferred = 0; slot.offset = offset; slot.buffer = buffer;