-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
memory leak in ModelSyncedEventEmitter
#3510
Comments
This issue was opened by a maintainer of this repository; updates will be posted here. If you are also experiencing this issue, please comment here with any relevant information so that we're aware and can prioritize accordingly. |
diff --git a/AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/InitialSync/ModelSyncedEventEmitter.swift b/AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/InitialSync/ModelSyncedEventEmitter.swift
index 9a44dbbc4..9bd806267 100644
--- a/AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/InitialSync/ModelSyncedEventEmitter.swift
+++ b/AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/InitialSync/ModelSyncedEventEmitter.swift
@@ -65,7 +65,7 @@ final class ModelSyncedEventEmitter {
self.syncOrchestratorSink = initialSyncOrchestrator?
.publisher
.receive(on: queue)
- .filter(filterSyncOperationEvent(_:))
+ .filter { [weak self] in self?.filterSyncOperationEvent($0) ?? false }
.sink(receiveCompletion: { _ in },
receiveValue: { [weak self] value in
self?.onReceiveSyncOperationEvent(value: value)
@@ -74,7 +74,7 @@ final class ModelSyncedEventEmitter {
self.reconciliationQueueSink = reconciliationQueue?
.publisher
.receive(on: queue)
- .filter(filterReconciliationQueueEvent(_:))
+ .filter { [weak self] in self?.filterReconciliationQueueEvent($0) ?? false }
.sink(receiveCompletion: { _ in },
receiveValue: { [weak self] value in
self?.onReceiveReconciliationEvent(value: value) |
Confirmed the memory leak does exist, and the proposed patch addresses and resolves the issue. Will created a PR for review and run tests. |
The patch has been released in version 2.33.4. Closing this ticket. |
This issue is now closed. Comments on closed issues are hard for our team to see. |
Also I found a small leak in
ModelSyncedEventEmitter
, you can find a patch for this here:memoryLeak.patch
This memory leak might be better tracked by another issue but as the code to easily reproduce it is here, I let you organise this as you want.
Originally posted by @gbitaudeau in #3259 (comment)
The text was updated successfully, but these errors were encountered: