Fix invalid watcher slot access in the win32 driver.

The underlying file handle and the associated slot could previously be destroyed before the I/O callback had been finally called. Now the callback itself is responsible to destroying the handle.
This commit is contained in:
Sönke Ludwig 2018-02-22 01:14:31 +01:00
parent c8f95f324b
commit 0e7091d085

View file

@ -50,6 +50,9 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers {
return WatcherID.invalid; return WatcherID.invalid;
} }
// keep alive as long as the overlapped I/O operation is pending
addRef(id);
m_core.addWaiter(); m_core.addWaiter();
return id; return id;
@ -62,16 +65,36 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers {
override bool releaseRef(WatcherID descriptor) override bool releaseRef(WatcherID descriptor)
{ {
auto handle = idToHandle(descriptor); return doReleaseRef(idToHandle(descriptor));
return m_core.m_handles[handle].releaseRef(()nothrow{ }
m_core.removeWaiter();
CloseHandle(handle); private static bool doReleaseRef(HANDLE handle)
() @trusted { {
try theAllocator.dispose(m_core.m_handles[handle].watcher.buffer); auto core = WinAPIEventDriver.threadInstance.core;
catch (Exception e) assert(false, "Freeing directory watcher buffer failed."); auto slot = () @trusted { return &core.m_handles[handle]; } ();
} ();
m_core.freeSlot(handle); if (!slot.releaseRef(() nothrow {
}); CloseHandle(handle);
() @trusted {
try theAllocator.dispose(slot.watcher.buffer);
catch (Exception e) assert(false, "Freeing directory watcher buffer failed.");
} ();
core.freeSlot(handle);
}))
{
return false;
}
// If only one reference left, then this is the reference created for
// the current wait operation. Simply cancel the I/O to let the
// completion callback
if (slot.refCount == 1) {
() @trusted { CancelIoEx(handle, &slot.watcher.overlapped); } ();
core.removeWaiter();
}
return true;
} }
private static nothrow extern(System) private static nothrow extern(System)
@ -81,30 +104,23 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers {
import std.file : isDir; import std.file : isDir;
import std.path : dirName, baseName, buildPath; import std.path : dirName, baseName, buildPath;
if (dwError != 0) {
// FIXME: this must be propagated to the caller
//logWarn("Failed to read directory changes: %s", dwError);
return;
}
auto handle = overlapped.hEvent; // *file* handle auto handle = overlapped.hEvent; // *file* handle
auto id = WatcherID(cast(int)handle); auto id = WatcherID(cast(int)handle);
/* HACK: this avoids a range voilation in case an already destroyed auto gslot = () @trusted { return &WinAPIEventDriver.threadInstance.core.m_handles[handle]; } ();
watcher still fires a completed event. It does not avoid problems auto slot = () @trusted { return &gslot.watcher(); } ();
that may arise from reused file handles.
*/ if (dwError != 0 || gslot.refCount == 1) {
if (handle !in WinAPIEventDriver.threadInstance.core.m_handles) // FIXME: error must be propagated to the caller (except for ABORTED
// errors)
//logWarn("Failed to read directory changes: %s", dwError);
doReleaseRef(handle);
return; return;
}
// NOTE: can be 0 if the buffer overflowed // NOTE: cbTransferred can be 0 if the buffer overflowed
if (!cbTransferred)
return;
auto slot = () @trusted { return &WinAPIEventDriver.threadInstance.core.m_handles[handle].watcher(); } ();
ubyte[] result = slot.buffer[0 .. cbTransferred]; ubyte[] result = slot.buffer[0 .. cbTransferred];
do { while (result.length) {
assert(result.length >= FILE_NOTIFY_INFORMATION._FileName.offsetof); assert(result.length >= FILE_NOTIFY_INFORMATION._FileName.offsetof);
auto fni = () @trusted { return cast(FILE_NOTIFY_INFORMATION*)result.ptr; } (); auto fni = () @trusted { return cast(FILE_NOTIFY_INFORMATION*)result.ptr; } ();
result = result[fni.NextEntryOffset .. $]; result = result[fni.NextEntryOffset .. $];
@ -130,7 +146,7 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers {
if (ch.kind != FileChangeKind.modified || !ch.isDirectory) if (ch.kind != FileChangeKind.modified || !ch.isDirectory)
slot.callback(id, ch); slot.callback(id, ch);
if (fni.NextEntryOffset == 0) break; if (fni.NextEntryOffset == 0) break;
} while (result.length > 0); }
triggerRead(handle, *slot); triggerRead(handle, *slot);
} }
@ -157,6 +173,7 @@ final class WinAPIEventDriverWatchers : EventDriverWatchers {
//logError("Failed to read directory changes in '%s'", m_path); //logError("Failed to read directory changes in '%s'", m_path);
return false; return false;
} }
return true; return true;
} }