From e2e8bf30aaec09329a9871117fe6e1ef27da93d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Fri, 26 Oct 2018 15:43:21 +0200 Subject: [PATCH] Change shutdown behavior to allow graceful handle leaks. This avoids crashing in case of any handle references left-over in GC allocated memory that gets finalized after module destructors have already been run. --- source/eventcore/core.d | 10 +++---- source/eventcore/driver.d | 9 +++++-- source/eventcore/drivers/posix/driver.d | 33 +++++++++++++++++++++-- source/eventcore/drivers/winapi/core.d | 25 ++++++++++++++++- source/eventcore/drivers/winapi/driver.d | 11 ++++++-- source/eventcore/drivers/winapi/events.d | 11 +++++++- source/eventcore/drivers/winapi/sockets.d | 10 +++++++ 7 files changed, 96 insertions(+), 13 deletions(-) diff --git a/source/eventcore/core.d b/source/eventcore/core.d index 8659998..db4dc5f 100644 --- a/source/eventcore/core.d +++ b/source/eventcore/core.d @@ -22,7 +22,7 @@ else alias NativeEventDriver = EventDriver; assert(s_driver !is null, "setupEventDriver() was not called for this thread."); else { if (!s_driver) { - s_driver = mallocT!NativeEventDriver(); + s_driver = mallocT!NativeEventDriver; } } return s_driver; @@ -43,8 +43,8 @@ static if (!is(NativeEventDriver == EventDriver)) { if (!s_isMainThread) { if (!--s_initCount) { if (s_driver) { - s_driver.dispose(); - freeT(s_driver); + if (s_driver.dispose()) + freeT(s_driver); } } } @@ -61,8 +61,8 @@ static if (!is(NativeEventDriver == EventDriver)) { { if (!--s_initCount) { if (s_driver) { - s_driver.dispose(); - freeT(s_driver); + if (s_driver.dispose()) + freeT(s_driver); } } } diff --git a/source/eventcore/driver.d b/source/eventcore/driver.d index aec1967..58fac0f 100644 --- a/source/eventcore/driver.d +++ b/source/eventcore/driver.d @@ -52,8 +52,13 @@ interface EventDriver { /// Directory change watching @property inout(EventDriverWatchers) watchers() inout; - /// Releases all resources associated with the driver - void dispose(); + /** Releases all resources associated with the driver. + + In case of any left-over referenced handles, this function returns + `false` and does not free any resources. It may choose to free the + resources once the last handle gets dereferenced. + */ + bool dispose(); } diff --git a/source/eventcore/drivers/posix/driver.d b/source/eventcore/drivers/posix/driver.d index 973b3db..2815fff 100644 --- a/source/eventcore/drivers/posix/driver.d +++ b/source/eventcore/drivers/posix/driver.d @@ -88,9 +88,29 @@ final class PosixEventDriver(Loop : PosixEventLoop) : EventDriver { final override @property inout(FileDriver) files() inout { return m_files; } final override @property inout(WatcherDriver) watchers() inout { return m_watchers; } - final override void dispose() + final override bool dispose() { - if (!m_loop) return; + import core.thread : Thread; + import taggedalgebraic : hasType; + + if (!m_loop) return true; + + static string getThreadName() + { + string thname; + try thname = Thread.getThis().name; + catch (Exception e) assert(false, e.msg); + return thname.length ? thname : "unknown"; + } + + if (m_loop.m_handleCount > 0) { + print("Warning (thread: %s): leaking eventcore driver because there are still active handles", getThreadName()); + foreach (id, ref s; m_loop.m_fds) + if (!s.specific.hasType!(typeof(null)) && !(s.common.flags & FDFlags.internal)) + print(" FD %s (%s)", id, s.specific.kind); + return false; + } + m_files.dispose(); m_dns.dispose(); m_core.dispose(); @@ -108,6 +128,8 @@ final class PosixEventDriver(Loop : PosixEventLoop) : EventDriver { freeT(m_loop); } (); catch (Exception e) assert(false, e.msg); + + return true; } } @@ -279,6 +301,7 @@ package class PosixEventLoop { package { AlgebraicChoppedVector!(FDSlot, StreamSocketSlot, StreamListenSocketSlot, DgramSocketSlot, DNSSlot, WatcherSlot, EventSlot, SignalSlot) m_fds; + size_t m_handleCount = 0; size_t m_waiterCount = 0; } @@ -338,6 +361,8 @@ package class PosixEventLoop { common.flags = flags; specific = slot_init; } + if (!(flags & FDFlags.internal)) + m_handleCount++; } package void clearFD(T)(FD fd) @@ -347,6 +372,10 @@ package class PosixEventLoop { auto slot = () @trusted { return &m_fds[fd.value]; } (); assert(slot.common.refCount == 0, "Clearing referenced file descriptor slot."); assert(slot.specific.hasType!T, "Clearing file descriptor slot with unmatched type."); + + if (!(slot.common.flags & FDFlags.internal)) + m_handleCount--; + if (slot.common.userDataDestructor) () @trusted { slot.common.userDataDestructor(slot.common.userData.ptr); } (); if (!(slot.common.flags & FDFlags.internal)) diff --git a/source/eventcore/drivers/winapi/core.d b/source/eventcore/drivers/winapi/core.d index 1c55fa8..6fcca7a 100644 --- a/source/eventcore/drivers/winapi/core.d +++ b/source/eventcore/drivers/winapi/core.d @@ -5,7 +5,7 @@ version (Windows): import eventcore.driver; import eventcore.drivers.timer; import eventcore.internal.consumablequeue; -import eventcore.internal.utils : mallocT, freeT, nogc_assert; +import eventcore.internal.utils : mallocT, freeT, nogc_assert, print; import eventcore.internal.win32; import core.sync.mutex : Mutex; import core.time : Duration; @@ -61,6 +61,29 @@ final class WinAPIEventDriverCore : EventDriverCore { } catch (Exception e) assert(false, e.msg); } + package bool checkForLeakedHandles() + @trusted { + import core.thread : Thread; + + static string getThreadName() + { + string thname; + try thname = Thread.getThis().name; + catch (Exception e) assert(false, e.msg); + return thname.length ? thname : "unknown"; + } + + foreach (k; m_handles.byKey) { + print("Warning (thread: %s): Leaked handles detected at driver shutdown", getThreadName()); + foreach (ks; m_handles.byKeyValue) + if (!ks.value.specific.hasType!(typeof(null))) + print(" FD %04X (%s)", ks.key, ks.value.specific.kind); + return true; + } + + return false; + } + override size_t waiterCount() { return m_waiterCount + m_timers.pendingCount; } package void addWaiter() @nogc { m_waiterCount++; } diff --git a/source/eventcore/drivers/winapi/driver.d b/source/eventcore/drivers/winapi/driver.d index d8bd0c3..47dd1f8 100644 --- a/source/eventcore/drivers/winapi/driver.d +++ b/source/eventcore/drivers/winapi/driver.d @@ -74,9 +74,14 @@ final class WinAPIEventDriver : EventDriver { override @property inout(WinAPIEventDriverSignals) signals() inout { return m_signals; } override @property inout(WinAPIEventDriverWatchers) watchers() inout { return m_watchers; } - override void dispose() + override bool dispose() { - if (!m_events) return; + if (!m_events) return true; + + if (m_core.checkForLeakedHandles()) return false; + if (m_events.checkForLeakedHandles()) return false; + if (m_sockets.checkForLeakedHandles()) return false; + m_events.dispose(); m_core.dispose(); assert(threadInstance !is null); @@ -93,5 +98,7 @@ final class WinAPIEventDriver : EventDriver { freeT(m_signals); } (); catch (Exception e) assert(false, e.msg); + + return true; } } diff --git a/source/eventcore/drivers/winapi/events.d b/source/eventcore/drivers/winapi/events.d index 0134c0c..731fff1 100644 --- a/source/eventcore/drivers/winapi/events.d +++ b/source/eventcore/drivers/winapi/events.d @@ -6,7 +6,7 @@ import eventcore.driver; import eventcore.drivers.winapi.core; import eventcore.internal.win32; import eventcore.internal.consumablequeue; -import eventcore.internal.utils : mallocT, freeT, nogc_assert; +import eventcore.internal.utils : mallocT, freeT, nogc_assert, print; final class WinAPIEventDriverEvents : EventDriverEvents { @@ -45,6 +45,15 @@ final class WinAPIEventDriverEvents : EventDriverEvents { freeT(m_pending); } + package bool checkForLeakedHandles() + @trusted { + foreach (evt; m_events.byKey) { + print("Warning: Event handles leaked at driver shutdown."); + return true; + } + return false; + } + override EventID create() { auto id = EventID(m_idCounter++); diff --git a/source/eventcore/drivers/winapi/sockets.d b/source/eventcore/drivers/winapi/sockets.d index 1263926..f4a8f11 100644 --- a/source/eventcore/drivers/winapi/sockets.d +++ b/source/eventcore/drivers/winapi/sockets.d @@ -18,6 +18,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets { private { alias SocketVector = AlgebraicChoppedVector!(SocketSlot, StreamSocketSlot, StreamListenSocketSlot, DatagramSocketSlot); SocketVector m_sockets; + size_t m_socketCount = 0; WinAPIEventDriverCore m_core; DWORD m_tid; HWND m_hwnd; @@ -38,6 +39,13 @@ final class WinAPIEventDriverSockets : EventDriverSockets { package @property size_t waiterCount() const { return m_waiters; } + package bool checkForLeakedHandles() + { + if (m_socketCount == 0) return false; + print("Warning: Socket handles leaked at driver shutdown."); + return true; + } + override StreamSocketFD connectStream(scope Address peer_address, scope Address bind_address, ConnectCallback on_connect) @trusted { assert(m_tid == GetCurrentThreadId()); @@ -802,6 +810,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets { private void initSocketSlot(SocketFD fd) { + m_socketCount++; m_sockets[fd.value].common.refCount = 1; m_sockets[fd.value].common.fd = fd; m_sockets[fd.value].common.driver = this; @@ -809,6 +818,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets { package void clearSocketSlot(FD fd) @nogc { + m_socketCount--; auto slot = () @trusted { return &m_sockets[fd]; } (); if (slot.common.userDataDestructor) () @trusted { slot.common.userDataDestructor(slot.common.userData.ptr); } ();