Merge pull request #69 from vibe-d/fix_file_close

Fix dangling handles resulting from actively closing a file.
This commit is contained in:
Sönke Ludwig 2018-03-18 01:31:40 +01:00 committed by GitHub
commit eaf7ab778b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 15 deletions

View file

@ -371,6 +371,14 @@ interface EventDriverFiles {
@safe: /*@nogc:*/ nothrow: @safe: /*@nogc:*/ nothrow:
FileFD open(string path, FileOpenMode mode); FileFD open(string path, FileOpenMode mode);
FileFD adopt(int system_file_handle); 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); void close(FileFD file);
ulong getSize(FileFD file); ulong getSize(FileFD file);

View file

@ -101,6 +101,7 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil
static struct FileInfo { static struct FileInfo {
IOInfo read; IOInfo read;
IOInfo write; IOInfo write;
bool open = true;
int refCount; int refCount;
DataInitializer userDataDestructor; DataInitializer userDataDestructor;
@ -175,7 +176,11 @@ final class ThreadedFileEventDriver(Events : EventDriverEvents) : EventDriverFil
void close(FileFD file) 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) 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) final override void write(FileFD file, ulong offset, const(ubyte)[] buffer, IOMode, FileIOCallback on_write_finish)
{ {
//assert(this.writable); //assert(this.writable);
auto f = &m_files[file].write; auto f = () @trusted { return &m_files[file]; } ();
if (!safeCAS(f.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated))
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(false, "Concurrent file writes are not allowed.");
assert(f.callback is null, "Concurrent file writes are not allowed."); assert(f.write.callback is null, "Concurrent file writes are not allowed.");
f.callback = on_write_finish; f.write.callback = on_write_finish;
m_activeWrites.insert(file); m_activeWrites.insert(file);
threadSetup(); threadSetup();
log("start write task"); 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) final override void read(FileFD file, ulong offset, ubyte[] buffer, IOMode, FileIOCallback on_read_finish)
{ {
auto f = &m_files[file].read; auto f = () @trusted { return &m_files[file]; } ();
if (!safeCAS(f.status, ThreadedFileStatus.idle, ThreadedFileStatus.initiated))
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(false, "Concurrent file reads are not allowed.");
assert(f.callback is null, "Concurrent file reads are not allowed."); assert(f.read.callback is null, "Concurrent file reads are not allowed.");
f.callback = on_read_finish; f.read.callback = on_read_finish;
m_activeReads.insert(file); m_activeReads.insert(file);
threadSetup(); threadSetup();
log("start read task"); log("start read task");

View file

@ -27,7 +27,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
auto access = mode == FileOpenMode.readWrite || mode == FileOpenMode.createTrunc ? (GENERIC_WRITE | GENERIC_READ) : auto access = mode == FileOpenMode.readWrite || mode == FileOpenMode.createTrunc ? (GENERIC_WRITE | GENERIC_READ) :
mode == FileOpenMode.append ? 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 creation = mode == FileOpenMode.createTrunc ? CREATE_ALWAYS : mode == FileOpenMode.append? OPEN_ALWAYS : OPEN_EXISTING;
auto handle = () @trusted { auto handle = () @trusted {
@ -76,7 +76,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
auto h = idToHandle(file); auto h = idToHandle(file);
auto slot = () @trusted { return &m_core.m_handles[h].file(); } (); auto slot = () @trusted { return &m_core.m_handles[h].file(); } ();
if (slot.read.overlapped.hEvent != INVALID_HANDLE_VALUE) { if (slot.read.overlapped.hEvent != INVALID_HANDLE_VALUE) {
CloseHandle(h);
slot.read.overlapped.hEvent = slot.write.overlapped.hEvent = INVALID_HANDLE_VALUE; 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) 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) { if (!buffer.length) {
on_write_finish(file, IOStatus.ok, 0); on_write_finish(file, IOStatus.ok, 0);
return; return;
} }
auto h = idToHandle(file);
auto slot = &m_core.m_handles[h].file.write;
slot.bytesTransferred = 0; slot.bytesTransferred = 0;
slot.offset = offset; slot.offset = offset;
slot.buffer = buffer; 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) 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) { if (!buffer.length) {
on_read_finish(file, IOStatus.ok, 0); on_read_finish(file, IOStatus.ok, 0);
return; return;
} }
auto h = idToHandle(file);
auto slot = &m_core.m_handles[h].file.read;
slot.bytesTransferred = 0; slot.bytesTransferred = 0;
slot.offset = offset; slot.offset = offset;
slot.buffer = buffer; slot.buffer = buffer;