Skip to content

Commit

Permalink
TW-1650: write ADRs for memory leak problems
Browse files Browse the repository at this point in the history
  • Loading branch information
sherlockvn authored and hoangdat committed May 8, 2024
1 parent 9fad9c1 commit e478298
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 1. Dispose ValueNotifier when not used anymore

Date: 2024-03-05

## Status

Accepted

## Context

In our UI code, ValueNotifier is frequently used for its simplicity and robustness in rebuilding widgets. However, post-utilization management of ValueNotifier is often neglected. Proper handling of ValueNotifier is crucial for stabilizing memory usage and enhancing app performance.

## Decision

Typically, ValueNotifiers are disposed of in the `dispose()` method within widget lifecycles. However, exceptions occur in scenarios involving interactors. Some interactors continue to use ValueNotifier even after yielding a state, due to their singleton nature. Disposing of ValueNotifier immediately after widget disposal can lead to exceptions because the interactor may still be active. To address this, we propose two solutions:
1. Dispose of the ValueNotifier when the interactor's activity concludes.
2. Refrain from using ValueNotifier in conjunction with interactors.

- Tickets references:
1. TW-1650: dispose ValueNotifier when not use anymore in video player
2. About the VideoPlayer (there are still some leakings, but its not critical right now - we wait for the owner's response - (here)[https://github.com/media-kit/media-kit/issues/776])
## Consequences

Implementing these practices ensures that ValueNotifiers are properly collected by the garbage collector, eliminating memory leaks associated with their misuse. This decision fosters more reliable and efficient memory management within our applications.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# 2. Cancel the stream subscription when not listen to its anymore

Date: 2024-03-05

## Status

Accepted

## Context

Our application extensively utilizes Streams for data handling. Given that some of these Streams are singleton and others are not, managing StreamSubscriptions is crucial. For instance, each Timeline object initiates three StreamSubscriptions. Without proper cancellation of these subscriptions, memory leaks occur.

## Decision

Specifically, within the `getTimeline()` method, Timeline objects are generated primarily for their callbacks, leading to the neglect of the actual objects. This oversight results in the leakage of three StreamSubscriptions per Timeline. Notably, during the initialization of the chat screen, `getTimeline()` is invoked multiple times, significantly increasing the risk of memory leaks. The resolved approach is to actively manage Timeline objects and ensure all associated StreamSubscriptions are canceled when they are no longer needed.

- Tickets references:
1. TW-1650: cancel StreamSubscription of timeline when not used anymore

## Consequences

By implementing this strategy, we substantially free up memory resources. Additionally, since some StreamSubscriptions are linked to substantial objects, managing these subscriptions effectively also releases these heavy objects from memory, enhancing overall application performance.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# 3. Handling Stream Subscriptions Without Nullifying Variables Post-Cancellation

Date: 2024-03-05

## Status

Accepted

## Context

The `cancel()` method for StreamSubscriptions is asynchronous. A common mistake is to set the variable holding the StreamSubscription to null immediately after calling `cancel()`. Doing so can render the cancellation ineffective, as the nullification may lead to the garbage collector prematurely collecting the subscription before the cancellation has completed.

## Decision

We will revise our approach to handling cancellations of StreamSubscriptions. Instead of nullifying the variable immediately, we will retain the reference until the cancellation has fully processed. This ensures the `cancel()` operation can complete effectively. Where possible, it is advisable to await the completion of `cancel()` to ensure proper resource management.

Ticket References:
1. TW-1650: remove the streamSubcription, listener in chat controller when not used any more
2. TW-1650: must await on cancel before remove item in list

## Consequences

By adopting this practice, we prevent potential memory leaks and ensure that StreamSubscriptions are properly disposed of. This adjustment in handling ensures more robust and reliable resource management within our applications.

0 comments on commit e478298

Please sign in to comment.