Improve robustness of file I/O callbacks for the WinAPI driver.

Explicitly gets the read/write slot from the handle map from within the callback instead of dereferencing the pointer assigned when starting the operation. This makes sure that no dangling reference is accessed. Instead, if the slot got reused before the callback is invoked (which is a bug), an assertion will be triggered if the slot now has a different handle type.
This commit is contained in:
Sönke Ludwig 2018-03-03 18:36:07 +01:00
parent 3a52e0f362
commit fbd6927377
2 changed files with 30 additions and 21 deletions

View file

@ -219,22 +219,20 @@ private struct HandleSlot {
package struct FileSlot {
static struct Direction(bool RO) {
OVERLAPPED overlapped;
WinAPIEventDriverCore core;
OVERLAPPED_FILE overlapped;
FileIOCallback callback;
ulong offset;
size_t bytesTransferred;
IOMode mode;
static if (RO) const(ubyte)[] buffer;
else ubyte[] buffer;
HANDLE handle; // set to INVALID_HANDLE_VALUE when closed
void invokeCallback(IOStatus status, size_t bytes_transferred)
@safe nothrow {
auto cb = this.callback;
this.callback = null;
assert(cb !is null);
cb(FileFD(cast(int)this.handle), status, bytes_transferred);
cb(overlapped.handle, status, bytes_transferred);
}
}
Direction!false read;
@ -248,3 +246,9 @@ package struct WatcherSlot {
bool recursive;
FileChangesCallback callback;
}
package struct OVERLAPPED_FILE {
OVERLAPPED overlapped;
WinAPIEventDriverCore driver;
FileFD handle;
}

View file

@ -62,7 +62,11 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
return FileFD.invalid;
auto s = m_core.setupSlot!FileSlot(handle);
s.read.handle = s.write.handle = handle;
s.read.overlapped.driver = m_core;
s.read.overlapped.handle = FileFD(cast(size_t)handle);
s.write.overlapped.driver = m_core;
s.write.overlapped.handle = FileFD(cast(size_t)handle);
return FileFD(cast(size_t)handle);
}
@ -71,9 +75,9 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
{
auto h = idToHandle(file);
auto slot = () @trusted { return &m_core.m_handles[h].file(); } ();
if (slot.read.handle != INVALID_HANDLE_VALUE) {
if (slot.read.overlapped.handle != FileFD.invalid) {
CloseHandle(h);
slot.read.handle = slot.write.handle = INVALID_HANDLE_VALUE;
slot.read.overlapped.handle = slot.write.overlapped.handle = FileFD.invalid;
}
}
@ -99,7 +103,6 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
slot.offset = offset;
slot.buffer = buffer;
slot.mode = mode;
slot.core = m_core;
slot.callback = on_write_finish;
m_core.addWaiter();
startIO!(WriteFileEx, true)(h, slot);
@ -118,7 +121,6 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
slot.offset = offset;
slot.buffer = buffer;
slot.mode = mode;
slot.core = m_core;
slot.callback = on_read_finish;
m_core.addWaiter();
startIO!(ReadFileEx, false)(h, slot);
@ -154,7 +156,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
{
import std.algorithm.comparison : min;
with (slot.overlapped) {
with (slot.overlapped.overlapped) {
Internal = 0;
InternalHigh = 0;
Offset = cast(uint)(slot.offset & 0xFFFFFFFF);
@ -163,8 +165,8 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
}
auto nbytes = min(slot.buffer.length, DWORD.max);
if (!() @trusted { return fun(h, &slot.buffer[0], nbytes, &slot.overlapped, &onIOFinished!(fun, RO)); } ()) {
slot.core.removeWaiter();
if (!() @trusted { return fun(h, &slot.buffer[0], nbytes, &slot.overlapped.overlapped, &onIOFinished!(fun, RO)); } ()) {
slot.overlapped.driver.removeWaiter();
slot.invokeCallback(IOStatus.error, slot.bytesTransferred);
}
}
@ -173,20 +175,23 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
{
if (slot.callback) {
m_core.removeWaiter();
() @trusted { CancelIoEx(h, &slot.overlapped); } ();
() @trusted { CancelIoEx(h, &slot.overlapped.overlapped); } ();
slot.callback = null;
slot.buffer = null;
}
}
private static extern(Windows)
void onIOFinished(alias fun, bool RO)(DWORD error, DWORD bytes_transferred, OVERLAPPED* overlapped)
void onIOFinished(alias fun, bool RO)(DWORD error, DWORD bytes_transferred, OVERLAPPED* _overlapped)
{
auto slot = () @trusted { return cast(FileSlot.Direction!RO*)overlapped.hEvent; } ();
auto ctx = () @trusted { return cast(OVERLAPPED_FILE*)_overlapped; } ();
FileFD id = ctx.handle;
auto handle = idToHandle(id);
static if (RO)
auto slot = () @trusted { return &ctx.driver.m_handles[handle].file.write; } ();
else
auto slot = () @trusted { return &ctx.driver.m_handles[handle].file.read; } ();
assert(slot !is null);
HANDLE h = slot.handle;
auto id = FileFD(cast(size_t)h);
if (!slot.callback) {
// request was already cancelled
@ -194,7 +199,7 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
}
if (error != 0) {
slot.core.removeWaiter();
ctx.driver.removeWaiter();
slot.invokeCallback(IOStatus.error, slot.bytesTransferred + bytes_transferred);
return;
}
@ -203,10 +208,10 @@ final class WinAPIEventDriverFiles : EventDriverFiles {
slot.offset += bytes_transferred;
if (slot.bytesTransferred >= slot.buffer.length || slot.mode != IOMode.all) {
slot.core.removeWaiter();
ctx.driver.removeWaiter();
slot.invokeCallback(IOStatus.ok, slot.bytesTransferred);
} else {
startIO!(fun, RO)(h, slot);
startIO!(fun, RO)(handle, slot);
}
}