Merge pull request #87 from vibe-d/graceful_handle_leak

Graceful handle leak at shutdown
merged-on-behalf-of: Sönke Ludwig <s-ludwig@users.noreply.github.com>

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,7 +43,7 @@ static if (!is(NativeEventDriver == EventDriver)) {
 		if (!s_isMainThread) {
 			if (!--s_initCount) {
 				if (s_driver) {
-					s_driver.dispose();
+					if (s_driver.dispose())
 						freeT(s_driver);
 				}
 			}
@@ -61,7 +61,7 @@ static if (!is(NativeEventDriver == EventDriver)) {
 	{
 		if (!--s_initCount) {
 			if (s_driver) {
-				s_driver.dispose();
+				if (s_driver.dispose())
 					freeT(s_driver);
 			}
 		}

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();
 }
 
 

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))

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++; }

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;
 	}
 }

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++);

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); } ();

source/eventcore/internal/utils.d

@@ -114,12 +114,12 @@ struct StdoutRange {
 
 	void put(string str)
 	{
-		() @trusted { fwrite(str.ptr, str.length, 1, stdout); } ();
+		() @trusted { fwrite(str.ptr, str.length, 1, stderr); } ();
 	}
 
 	void put(char ch)
 	{
-		() @trusted { fputc(ch, stdout); } ();
+		() @trusted { fputc(ch, stderr); } ();
 	}
 }
 

tests/0-leaked-handles.d

@@ -0,0 +1,36 @@
+/++ dub.sdl:
+	name "test"
+	dependency "eventcore" path=".."
++/
+module test;
+
+import eventcore.core;
+import eventcore.driver;
+import std.socket : InternetAddress;
+
+
+class C {
+	DatagramSocketFD m_handle;
+	EventDriver m_driver;
+
+	this()
+	{
+		auto addr = new InternetAddress(0x7F000001, 40001);
+		m_handle = eventDriver.sockets.createDatagramSocket(addr, null);
+		assert(m_handle != DatagramSocketFD.invalid);
+		m_driver = eventDriver;
+	}
+
+	~this()
+	{
+		assert(eventDriver is m_driver);
+		eventDriver.sockets.releaseRef(m_handle);
+	}
+}
+
+void main()
+{
+	// let the GC clean up at app exit
+	// note that this happens *after* the static module destructors have been run
+	new C;
+}