From 2ad90333aa4e0bdf9ccbbe3b79fce7ca5dc8376a Mon Sep 17 00:00:00 2001 From: Patrick Freed Date: Wed, 27 May 2020 13:18:36 -0400 Subject: [PATCH 1/2] clarify when cursors/change streams need to be killed --- Sources/MongoSwift/ChangeStream.swift | 13 ++++++++++++- Sources/MongoSwift/MongoClient.swift | 12 ++++++++++++ .../MongoCollection+ChangeStreams.swift | 12 ++++++++++++ .../MongoSwift/MongoCollection+Indexes.swift | 4 ++++ Sources/MongoSwift/MongoCollection+Read.swift | 8 ++++++++ Sources/MongoSwift/MongoCursor.swift | 19 +++++++++++++++---- Sources/MongoSwift/MongoDatabase.swift | 16 ++++++++++++++++ Sources/MongoSwiftSync/ChangeStream.swift | 6 ++++-- Sources/MongoSwiftSync/MongoCursor.swift | 8 ++++++-- 9 files changed, 89 insertions(+), 9 deletions(-) diff --git a/Sources/MongoSwift/ChangeStream.swift b/Sources/MongoSwift/ChangeStream.swift index ea2ca5fb7..ad100eb14 100644 --- a/Sources/MongoSwift/ChangeStream.swift +++ b/Sources/MongoSwift/ChangeStream.swift @@ -108,7 +108,18 @@ public class ChangeStream: CursorProtocol { } } - /// Indicates whether this change stream has the potential to return more data. + /** + * Indicates whether this change stream has the potential to return more data. + * + * This change stream will be dead if `next` returns `nil`, but it may still be alive if `tryNext` returns `nil`. + * + * If either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may + * still be alive if either returns a `DecodingError`, however. + * + * - Warning: + * If this change stream is alive when it goes out of scope, it will leak resources. To ensure it is dead + * before it leaves scope, invoke `ChangeStream.kill(...)` on it. + */ public func isAlive() -> EventLoopFuture { self.client.operationExecutor.execute { self.wrappedCursor.isAlive diff --git a/Sources/MongoSwift/MongoClient.swift b/Sources/MongoSwift/MongoClient.swift index 84651e509..ba3e4c847 100644 --- a/Sources/MongoSwift/MongoClient.swift +++ b/Sources/MongoSwift/MongoClient.swift @@ -475,6 +475,10 @@ public class MongoClient { * - options: An optional `ChangeStreamOptions` to use when constructing the change stream. * - session: An optional `ClientSession` to use with this change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * deployment. @@ -512,6 +516,10 @@ public class MongoClient { * - withFullDocumentType: The type that the `fullDocument` field of the emitted `ChangeStreamEvent`s will be * decoded to. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * deployment. @@ -554,6 +562,10 @@ public class MongoClient { * - withEventType: The type that the entire change stream response will be decoded to and that will be returned * when iterating through the change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * deployment. diff --git a/Sources/MongoSwift/MongoCollection+ChangeStreams.swift b/Sources/MongoSwift/MongoCollection+ChangeStreams.swift index 3a7f64a5f..c814b0c93 100644 --- a/Sources/MongoSwift/MongoCollection+ChangeStreams.swift +++ b/Sources/MongoSwift/MongoCollection+ChangeStreams.swift @@ -12,6 +12,10 @@ extension MongoCollection { * - options: An optional `ChangeStreamOptions` to use when constructing the change stream. * - session: An optional `ClientSession` to use with this change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching this collection. * @@ -46,6 +50,10 @@ extension MongoCollection { * - withFullDocumentType: The type that the `fullDocument` field of the emitted `ChangeStreamEvent`s will be * decoded to. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching this collection. * @@ -85,6 +93,10 @@ extension MongoCollection { * - withEventType: The type that the entire change stream response will be decoded to and that will be returned * when iterating through the change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching this collection. * diff --git a/Sources/MongoSwift/MongoCollection+Indexes.swift b/Sources/MongoSwift/MongoCollection+Indexes.swift index 7e59ee2db..1a874f1c0 100644 --- a/Sources/MongoSwift/MongoCollection+Indexes.swift +++ b/Sources/MongoSwift/MongoCollection+Indexes.swift @@ -381,6 +381,10 @@ extension MongoCollection { * - Parameters: * - session: Optional `ClientSession` to use when executing this command * + * - Warning: + * If the returned cursor is alive when it goes out of scope, it will leak resources. To ensure the + * cursor is dead before it leaves scope, invoke `MongoCursor.kill(...)` on it. + * * - Returns: * An `EventLoopFuture>`. On success, contains a cursor over the indexes. * diff --git a/Sources/MongoSwift/MongoCollection+Read.swift b/Sources/MongoSwift/MongoCollection+Read.swift index 0059bee62..2c3a3126e 100644 --- a/Sources/MongoSwift/MongoCollection+Read.swift +++ b/Sources/MongoSwift/MongoCollection+Read.swift @@ -6,6 +6,10 @@ extension MongoCollection { /** * Finds the documents in this collection which match the provided filter. * + * - Warning: + * If the returned cursor is alive when it goes out of scope, it will leak resources. To ensure the cursor + * is dead before it leaves scope, invoke `MongoCursor.kill(...)` on it. + * * - Parameters: * - filter: A `BSONDocument` that should match the query * - options: Optional `FindOptions` to use when executing the command @@ -66,6 +70,10 @@ extension MongoCollection { * - options: Optional `AggregateOptions` to use when executing the command * - session: Optional `ClientSession` to use when executing this command * + * - Warning: + * If the returned cursor is alive when it goes out of scope, it will leak resources. To ensure the cursor + * is dead before it leaves scope, invoke `MongoCursor.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a cursor over the resulting documents. * diff --git a/Sources/MongoSwift/MongoCursor.swift b/Sources/MongoSwift/MongoCursor.swift index 95f364728..5b6730388 100644 --- a/Sources/MongoSwift/MongoCursor.swift +++ b/Sources/MongoSwift/MongoCursor.swift @@ -99,9 +99,17 @@ public class MongoCursor: CursorProtocol { * This method is mainly useful if this cursor is tailable, since in that case `tryNext` may return more results * even after returning `nil`. * - * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or an error. + * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or a + * non-`DecodingError` error. * - * This cursor will be dead as soon as `next` returns `nil` or an error, regardless of the `MongoCursorType`. + * This cursor will be dead as soon as `next` returns `nil` or a non-`DecodingError` error, regardless of the + * `MongoCursorType`. + * + * This cursor may still be alive after `next` or `tryNext` returns a `DecodingError`. + * + * - Warning: + * If this cursor is alive when it goes out of scope, it will leak resources. To ensure it is dead before it + * leaves scope, invoke `MongoCursor.kill(...)` on it. */ public func isAlive() -> EventLoopFuture { self.client.operationExecutor.execute { @@ -231,8 +239,11 @@ public class MongoCursor: CursorProtocol { /** * Kill this cursor. * - * This method MUST be called before this cursor goes out of scope to prevent leaking resources. - * This method may be called even if there are unresolved futures created from other `MongoCursor` methods. + * This method MUST be called before any alive cursor goes out of scope to prevent leaking resources. + * + * This method MAY be called even if there are unresolved futures created from other `MongoCursor` methods. + * + * This method MAY be called if the cursor is already dead. It will have no effect. * * - Returns: * An `EventLoopFuture` that evaluates when the cursor has completed closing. This future should not fail. diff --git a/Sources/MongoSwift/MongoDatabase.swift b/Sources/MongoSwift/MongoDatabase.swift index 666995eef..9e3deb02a 100644 --- a/Sources/MongoSwift/MongoDatabase.swift +++ b/Sources/MongoSwift/MongoDatabase.swift @@ -234,6 +234,10 @@ public struct MongoDatabase { * - options: Optional `ListCollectionsOptions` to use when executing this command * - session: Optional `ClientSession` to use when executing this command * + * - Warning: + * If the returned cursor is alive when it goes out of scope, it will leak resources. To ensure the cursor + * is dead before it leaves scope, invoke `MongoCursor.kill(...)` on it. + * * - Returns: * An `EventLoopFuture>` containing a cursor over the collections. * @@ -352,6 +356,10 @@ public struct MongoDatabase { * - options: An optional `ChangeStreamOptions` to use when constructing the change stream. * - session: An optional `ClientSession` to use with this change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * database. @@ -389,6 +397,10 @@ public struct MongoDatabase { * - withFullDocumentType: The type that the `fullDocument` field of the emitted `ChangeStreamEvent`s will be * decoded to. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * database. @@ -431,6 +443,10 @@ public struct MongoDatabase { * - withEventType: The type that the entire change stream response will be decoded to and that will be returned * when iterating through the change stream. * + * - Warning: + * If the returned change stream is alive when it goes out of scope, it will leak resources. To ensure the + * change stream is dead before it leaves scope, invoke `ChangeStream.kill(...)` on it. + * * - Returns: * An `EventLoopFuture`. On success, contains a `ChangeStream` watching all collections in this * database. diff --git a/Sources/MongoSwiftSync/ChangeStream.swift b/Sources/MongoSwiftSync/ChangeStream.swift index 12c40c83e..c5b9aae08 100644 --- a/Sources/MongoSwiftSync/ChangeStream.swift +++ b/Sources/MongoSwiftSync/ChangeStream.swift @@ -26,8 +26,10 @@ public class ChangeStream: CursorProtocol { /** * Indicates whether this change stream has the potential to return more data. * - * This change stream will be dead if `next` returns `nil` or an error. It will also be dead if `tryNext` returns - * an error, but will still be alive if `tryNext` returns `nil`. + * This change stream will be dead if `next` returns `nil`, but it may still be alive if `tryNext` returns `nil`. + * + * If either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may + * still be alive if either returns a `DecodingError`, however. */ public func isAlive() -> Bool { do { diff --git a/Sources/MongoSwiftSync/MongoCursor.swift b/Sources/MongoSwiftSync/MongoCursor.swift index 8cca1ee52..50e910940 100644 --- a/Sources/MongoSwiftSync/MongoCursor.swift +++ b/Sources/MongoSwiftSync/MongoCursor.swift @@ -19,9 +19,13 @@ public class MongoCursor: CursorProtocol { * This method is mainly useful if this cursor is tailable, since in that case `tryNext` may return more results * even after returning `nil`. * - * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or an error. + * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or a + * non-`DecodingError` error. * - * This cursor will be dead as soon as `next` returns `nil` or an error, regardless of the `MongoCursorType`. + * This cursor will be dead as soon as `next` returns `nil` or a non-`DecodingError` error, regardless of the + * `MongoCursorType`. + * + * This cursor may still be alive after `next` or `tryNext` returns a `DecodingError`. */ public func isAlive() -> Bool { do { From 52deb8e435bea7519b116889376a0573db953f01 Mon Sep 17 00:00:00 2001 From: Patrick Freed Date: Wed, 27 May 2020 13:49:18 -0400 Subject: [PATCH 2/2] minor improvements --- Sources/MongoSwift/ChangeStream.swift | 17 +++++++++++------ Sources/MongoSwift/MongoCursor.swift | 10 ++++++---- Sources/MongoSwiftSync/ChangeStream.swift | 7 ++++--- Sources/MongoSwiftSync/MongoCursor.swift | 4 ++-- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/Sources/MongoSwift/ChangeStream.swift b/Sources/MongoSwift/ChangeStream.swift index ad100eb14..84b7c9ecd 100644 --- a/Sources/MongoSwift/ChangeStream.swift +++ b/Sources/MongoSwift/ChangeStream.swift @@ -111,10 +111,11 @@ public class ChangeStream: CursorProtocol { /** * Indicates whether this change stream has the potential to return more data. * - * This change stream will be dead if `next` returns `nil`, but it may still be alive if `tryNext` returns `nil`. + * This change stream will be dead after `next` returns `nil`, but it may still be alive after `tryNext` returns + * `nil`. * - * If either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may - * still be alive if either returns a `DecodingError`, however. + * After either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may + * still be alive after either returns a `DecodingError`, however. * * - Warning: * If this change stream is alive when it goes out of scope, it will leak resources. To ensure it is dead @@ -245,9 +246,13 @@ public class ChangeStream: CursorProtocol { /** * Kill this change stream. * - * This method MUST be called before this change stream goes out of scope to prevent leaking resources. - * This method may be called even if there are unresolved futures created from other `ChangeStream` methods. - * This method will have no effect if the change stream is already dead. + * This method MAY be called even if there are unresolved futures created from other `ChangeStream` methods. + * + * This method MAY be called if the change stream is already dead. It will have no effect. + * + * - Warning: + * If this change stream is alive when it goes out of scope, it will leak resources. To ensure it + * is dead before it leaves scope, invoke this method. * * - Returns: * An `EventLoopFuture` that evaluates when the change stream has completed closing. This future should not fail. diff --git a/Sources/MongoSwift/MongoCursor.swift b/Sources/MongoSwift/MongoCursor.swift index 5b6730388..03fd9ff23 100644 --- a/Sources/MongoSwift/MongoCursor.swift +++ b/Sources/MongoSwift/MongoCursor.swift @@ -99,10 +99,10 @@ public class MongoCursor: CursorProtocol { * This method is mainly useful if this cursor is tailable, since in that case `tryNext` may return more results * even after returning `nil`. * - * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or a + * If this cursor is non-tailable, it will always be dead after either `tryNext` returns `nil` or a * non-`DecodingError` error. * - * This cursor will be dead as soon as `next` returns `nil` or a non-`DecodingError` error, regardless of the + * This cursor will be dead after `next` returns `nil` or a non-`DecodingError` error, regardless of the * `MongoCursorType`. * * This cursor may still be alive after `next` or `tryNext` returns a `DecodingError`. @@ -239,12 +239,14 @@ public class MongoCursor: CursorProtocol { /** * Kill this cursor. * - * This method MUST be called before any alive cursor goes out of scope to prevent leaking resources. - * * This method MAY be called even if there are unresolved futures created from other `MongoCursor` methods. * * This method MAY be called if the cursor is already dead. It will have no effect. * + * - Warning: + * If this cursor is alive when it goes out of scope, it will leak resources. To ensure it + * is dead before it leaves scope, invoke this method. + * * - Returns: * An `EventLoopFuture` that evaluates when the cursor has completed closing. This future should not fail. */ diff --git a/Sources/MongoSwiftSync/ChangeStream.swift b/Sources/MongoSwiftSync/ChangeStream.swift index c5b9aae08..4e1426a5c 100644 --- a/Sources/MongoSwiftSync/ChangeStream.swift +++ b/Sources/MongoSwiftSync/ChangeStream.swift @@ -26,10 +26,11 @@ public class ChangeStream: CursorProtocol { /** * Indicates whether this change stream has the potential to return more data. * - * This change stream will be dead if `next` returns `nil`, but it may still be alive if `tryNext` returns `nil`. + * This change stream will be dead after `next` returns `nil`, but it may still be alive after `tryNext` returns + * `nil`. * - * If either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may - * still be alive if either returns a `DecodingError`, however. + * After either of `next` or `tryNext` return a non-`DecodingError` error, this change stream will be dead. It may + * still be alive after either returns a `DecodingError`, however. */ public func isAlive() -> Bool { do { diff --git a/Sources/MongoSwiftSync/MongoCursor.swift b/Sources/MongoSwiftSync/MongoCursor.swift index 50e910940..e1fea7c76 100644 --- a/Sources/MongoSwiftSync/MongoCursor.swift +++ b/Sources/MongoSwiftSync/MongoCursor.swift @@ -19,10 +19,10 @@ public class MongoCursor: CursorProtocol { * This method is mainly useful if this cursor is tailable, since in that case `tryNext` may return more results * even after returning `nil`. * - * If this cursor is non-tailable, it will always be dead as soon as either `tryNext` returns `nil` or a + * If this cursor is non-tailable, it will always be dead after either `tryNext` returns `nil` or a * non-`DecodingError` error. * - * This cursor will be dead as soon as `next` returns `nil` or a non-`DecodingError` error, regardless of the + * This cursor will be dead after `next` returns `nil` or a non-`DecodingError` error, regardless of the * `MongoCursorType`. * * This cursor may still be alive after `next` or `tryNext` returns a `DecodingError`.