From ddd5225c9d23e451d6975d87018505f8a72d75e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Mon, 25 Mar 2024 08:33:42 +0100 Subject: [PATCH 1/3] Avoid possible GC allocations in MongoCursor.~this. Fixes #2793. This adds a check to the destructor to avoid running into an InvalidMemoryOperationError and instead outputs a more descriptive error message without crashing. This also swaps back the order of operations in killCursors() so that no connection to the server gets allocated by the call if the cursor has already been killed. --- mongodb/vibe/db/mongo/cursor.d | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/mongodb/vibe/db/mongo/cursor.d b/mongodb/vibe/db/mongo/cursor.d index ff855e9f1..79d1203e4 100644 --- a/mongodb/vibe/db/mongo/cursor.d +++ b/mongodb/vibe/db/mongo/cursor.d @@ -119,9 +119,22 @@ struct MongoCursor(DocType = Bson) { ~this() @safe { + import core.memory : GC; + if( m_data && --m_data.refCount == 0 ){ try { - m_data.killCursors(); + // avoid InvalidMemoryOperation errors in case the cursor was + // leaked to the GC + if (m_data.alive && GC.inFinalizer) { + logError("MongoCursor instance that has not been fully processed leaked to the GC!"); + try throw new Exception(""); + catch (Exception e) { + try () @trusted { logError("%s", e.info); } (); + catch (Exception e2) logError(" ... failed to generate stack trace"); + } + } else { + m_data.killCursors(); + } } catch (MongoException e) { logWarn("MongoDB failed to kill cursors: %s", e.msg); logDiagnostic("%s", (() @trusted => e.toString)()); @@ -288,6 +301,7 @@ struct MongoCursor(DocType = Bson) { /// interface because we still have code for legacy (<3.6) MongoDB servers, /// which may still used with the old legacy overloads. private interface IMongoCursorData(DocType) { + @property bool alive() @safe nothrow; bool empty() @safe; /// Range implementation long index() @safe; /// Range implementation DocType front() @safe; /// Range implementation @@ -326,6 +340,8 @@ private deprecated abstract class LegacyMongoCursorData(DocType) : IMongoCursorD long m_limit = 0; } + @property bool alive() @safe nothrow { return m_cursor != 0; } + final bool empty() @safe { if (!m_iterationStarted) startIterating(); @@ -390,8 +406,8 @@ private deprecated abstract class LegacyMongoCursorData(DocType) : IMongoCursorD final void killCursors() @safe { - auto conn = m_client.lockConnection(); if (m_cursor == 0) return; + auto conn = m_client.lockConnection(); conn.killCursors(m_collection, () @trusted { return (&m_cursor)[0 .. 1]; } ()); m_cursor = 0; } @@ -448,6 +464,8 @@ private class MongoFindCursor(DocType) : IMongoCursorData!DocType { m_database = command["$db"].opt!string; } + @property bool alive() @safe nothrow { return m_cursor != 0; } + bool empty() @safe { if (!m_iterationStarted) startIterating(); @@ -515,8 +533,8 @@ private class MongoFindCursor(DocType) : IMongoCursorData!DocType { final void killCursors() @safe { - auto conn = m_client.lockConnection(); if (m_cursor == 0) return; + auto conn = m_client.lockConnection(); conn.killCursors(m_ns, () @trusted { return (&m_cursor)[0 .. 1]; } ()); m_cursor = 0; } From 4bb596641d51583aae65786a545c77c8a1d14cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Mon, 25 Mar 2024 08:36:35 +0100 Subject: [PATCH 2/3] Reorder destructor code in a more logical way. --- mongodb/vibe/db/mongo/cursor.d | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/mongodb/vibe/db/mongo/cursor.d b/mongodb/vibe/db/mongo/cursor.d index 79d1203e4..ad96b3844 100644 --- a/mongodb/vibe/db/mongo/cursor.d +++ b/mongodb/vibe/db/mongo/cursor.d @@ -121,11 +121,11 @@ struct MongoCursor(DocType = Bson) { { import core.memory : GC; - if( m_data && --m_data.refCount == 0 ){ - try { + if (m_data && --m_data.refCount == 0) { + if (m_data.alive) { // avoid InvalidMemoryOperation errors in case the cursor was // leaked to the GC - if (m_data.alive && GC.inFinalizer) { + if(GC.inFinalizer) { logError("MongoCursor instance that has not been fully processed leaked to the GC!"); try throw new Exception(""); catch (Exception e) { @@ -133,11 +133,12 @@ struct MongoCursor(DocType = Bson) { catch (Exception e2) logError(" ... failed to generate stack trace"); } } else { - m_data.killCursors(); + try m_data.killCursors(); + catch (MongoException e) { + logWarn("MongoDB failed to kill cursors: %s", e.msg); + logDiagnostic("%s", (() @trusted => e.toString)()); + } } - } catch (MongoException e) { - logWarn("MongoDB failed to kill cursors: %s", e.msg); - logDiagnostic("%s", (() @trusted => e.toString)()); } } } From da4b61d6062d8e77fb022ca3a1d73eeb4c83d692 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Mon, 25 Mar 2024 20:54:40 +0100 Subject: [PATCH 3/3] Remove stack trace code that itself would trigger an InvalidMemoryOperationError. --- mongodb/vibe/db/mongo/cursor.d | 5 ----- 1 file changed, 5 deletions(-) diff --git a/mongodb/vibe/db/mongo/cursor.d b/mongodb/vibe/db/mongo/cursor.d index ad96b3844..2e5d5eb76 100644 --- a/mongodb/vibe/db/mongo/cursor.d +++ b/mongodb/vibe/db/mongo/cursor.d @@ -127,11 +127,6 @@ struct MongoCursor(DocType = Bson) { // leaked to the GC if(GC.inFinalizer) { logError("MongoCursor instance that has not been fully processed leaked to the GC!"); - try throw new Exception(""); - catch (Exception e) { - try () @trusted { logError("%s", e.info); } (); - catch (Exception e2) logError(" ... failed to generate stack trace"); - } } else { try m_data.killCursors(); catch (MongoException e) {