[BUG] possible memory leak in AsyncBufferedChannel

[lhoward]

Still figuring out whether this is a bug in my code or AsyncExtensions. I'm seeing a bunch of small leaks from AsyncBufferedChannel when used by AsyncCurrentValueSubject. Note in order to aid debugging I've modified the code to use checked continuations, although I will revert this once it is fixed.

More information as I have it, just wondering if anyone else has seen this.

relates to internal bug PADL/inferno_ui#15

STACK OF 1 INSTANCE OF 'ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<SwiftOCA.OcaProperty<SwiftOCA.OcaBoundedPropertyValue<Swift.Float>>.PropertyValue<>>.Awaiting<>>>':
18  libsystem_pthread.dylib               0x19d069d28 start_wqthread + 8
17  libsystem_pthread.dylib               0x19d06afd0 _pthread_wqthread + 228
16  libdispatch.dylib                     0x19ced06b8 _dispatch_worker_thread2 + 156
15  libdispatch.dylib                     0x19cecfea8 _dispatch_root_queue_drain + 392
14  libswift_Concurrency.dylib            0x2651dc470 swift_job_runImpl(swift::Job*, swift::ExecutorRef) + 72
13  libswift_Concurrency.dylib            0x2651db2e0 swift::runJobInEstablishedExecutorContext(swift::Job*) + 436
12  ocacli                                0x100f82e38 back deployment fallback for withCheckedContinuation<A>(isolation:function:_:) + 144  /<compiler-generated>:0
11  ocacli                                0x100f82fac closure #1 in withCheckedContinuation<A>(isolation:function:_:) + 220  /<compiler-generated>:0
10  ocacli                                0x100f7d8bc closure #1 in closure #2 in AsyncBufferedChannel.next(onSuspend:) + 728  AsyncBufferedChannel.swift:182
9   ocacli                                0x100ff9610 ManagedCriticalState.withCriticalRegion<A>(_:) + 188  Locking.swift:136
8   ocacli                                0x100f306a4 $ss13ManagedBufferCsRi__rlE25withUnsafeMutablePointersyqd_0_qd_0_SpyxG_Spyq_Gtqd__YKXEqd__YKs5ErrorRd__Ri_d_0_r0_lF + 172  /<compiler-generated>:0
7   ocacli                                0x100ff977c partial apply for closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 52  /<compiler-generated>:0
6   ocacli                                0x100ff96e0 closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 124  Locking.swift:139
5   ocacli                                0x100f83000 partial apply for closure #1 in closure #1 in closure #2 in AsyncBufferedChannel.next(onSuspend:) + 44  /<compiler-generated>:0
4   ocacli                                0x100f7e0d8 closure #1 in closure #1 in closure #2 in AsyncBufferedChannel.next(onSuspend:) + 1040  AsyncBufferedChannel.swift:188
3   libswiftCore.dylib                    0x1acd63c10 _allocateUninitializedArray<A>(_:) + 144
2   libswiftCore.dylib                    0x1ad0e27b8 swift_allocObject + 64
1   libswiftCore.dylib                    0x1ad0e24cc swift_slowAlloc + 36
0   libsystem_malloc.dylib                0x19cea8adc _malloc_zone_malloc_instrumented_or_legacy + 264 
====

[lhoward]

The leaked value is this (line 188 in my branch):

            case .idle:
              state = .awaiting([Awaiting(id: awaitingId, continuation: continuation)])
              return .suspend

[lhoward]

Also, I suspect moving to Mutex in Synchronization might be worth it as it will save a bunch of mallocs, as will replacing generateId() with an atomic.

[lhoward]

Another data point: we are using an AsyncCurrentValueSubject for audio metering information which is updated 60 times per second. So if there's a race, this will likely trigger it.

[lhoward]

I also have the PR from #40 in my branch FYI (this didn't improve things though).

[lhoward]

I can see this also when running leaks on the test suite:

18  libsystem_pthread.dylib               0x19d069d28 start_wqthread + 8
17  libsystem_pthread.dylib               0x19d06afd0 _pthread_wqthread + 228
16  libdispatch.dylib                     0x19ced06b8 _dispatch_worker_thread2 + 156
15  libdispatch.dylib                     0x19cecfea8 _dispatch_root_queue_drain + 392
14  libswift_Concurrency.dylib            0x2651dc470 swift_job_runImpl(swift::Job*, swift::ExecutorRef) + 72
13  libswift_Concurrency.dylib            0x2651db2e0 swift::runJobInEstablishedExecutorContext(swift::Job*) + 436
12  AsyncExtensionsPackageTests           0x1023971dc back deployment fallback for withCheckedContinuation<A>(isolation:function:_:) + 144  /<compiler-generated>:0
11  AsyncExtensionsPackageTests           0x102397350 closure #1 in withCheckedContinuation<A>(isolation:function:_:) + 220  /<compiler-generated>:0
10  AsyncExtensionsPackageTests           0x102391564 closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 708  AsyncBufferedChannel.swift:160
9   AsyncExtensionsPackageTests           0x10240d2c4 ManagedCriticalState.withCriticalRegion<A>(_:) + 188  Locking.swift:136
8   AsyncExtensionsPackageTests           0x102344a50 $ss13ManagedBufferCsRi__rlE25withUnsafeMutablePointersyqd_0_qd_0_SpyxG_Spyq_Gtqd__YKXEqd__YKs5ErrorRd__Ri_d_0_r0_lF + 172  /<compiler-generated>:0
7   AsyncExtensionsPackageTests           0x10240d430 partial apply for closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 52  /<compiler-generated>:0
6   AsyncExtensionsPackageTests           0x10240d394 closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 124  Locking.swift:139
5   AsyncExtensionsPackageTests           0x1023973a4 partial apply for closure #1 in closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 44  /<compiler-generated>:0
4   AsyncExtensionsPackageTests           0x102391ae4 closure #1 in closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 976  AsyncBufferedChannel.swift:166
3   libswiftCore.dylib                    0x1acd63c10 _allocateUninitializedArray<A>(_:) + 144
2   libswiftCore.dylib                    0x1ad0e27b8 swift_allocObject + 64
1   libswiftCore.dylib                    0x1ad0e24cc swift_slowAlloc + 36
0   libsystem_malloc.dylib                0x19cea8adc _malloc_zone_malloc_instrumented_or_legacy + 264 
====
    32 (7.72K) << TOTAL >>
      ----
      11 (3.62K) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x600000959620> [48]
         10 (3.58K) <CheckedContinuationCanary 0x600000959b00> [48]
            9 (3.53K) __strong _refcounts + 8 --> <Task stack 0x136012d90> [288]
               5 (2.14K) <Task stack 0x135834400> [1024]
                  2 (1.03K) <Task stack 0x135843000> [1024]
                     1 (32 bytes) <Atomics.ManagedAtomic<Swift.Bool> 0x6000006e4ca0> [32]
                  1 (64 bytes) <Swift._ContiguousArrayStorage<Swift.Int> 0x600001213800> [64]
                  1 (48 bytes) <malloc in AsyncReplaySubject.handleNewConsumer() 0x600000959bc0> [48]
               1 (1.00K) <Task stack 0x135843400> [1024]
               2 (112 bytes) <Closure context (unknown layout) 0x600000959320> [48]
                  1 (64 bytes) <Closure context 0x600001214300> [64]
      ----
      11 (3.62K) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x600000978f90> [48]
         10 (3.58K) <CheckedContinuationCanary 0x600000979110> [48]
            9 (3.53K) __strong _refcounts + 8 --> <Task stack 0x134704dc0> [288]
               5 (2.14K) <Task stack 0x13582ca00> [1024]
                  2 (1.03K) <Task stack 0x135041c00> [1024]
                     1 (32 bytes) <Atomics.ManagedAtomic<Swift.Bool> 0x6000006e0840> [32]
                  1 (64 bytes) <Swift._ContiguousArrayStorage<Swift.Int> 0x600001278340> [64]
                  1 (48 bytes) <malloc in AsyncCurrentValueSubject.handleNewConsumer() 0x600000979680> [48]
               1 (1.00K) <Task stack 0x13500cc00> [1024]
               2 (112 bytes) <Closure context (unknown layout) 0x600000979650> [48]
                  1 (64 bytes) <Closure context 0x600001279080> [64]
      ----
      2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x60000095a160> [48]
         1 (48 bytes) <CheckedContinuationCanary 0x60000095a100> [48]
      ----
      2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x60000095c210> [48]
         1 (48 bytes) <CheckedContinuationCanary 0x60000095c150> [48]
      ----
      2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x60000095c4e0> [48]
         1 (48 bytes) <CheckedContinuationCanary 0x60000095c4b0> [48]
      ----
      2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x60000096f600> [48]
         1 (48 bytes) <CheckedContinuationCanary 0x60000096f570> [48]
      ----
      2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x60000097c900> [48]
         1 (48 bytes) <CheckedContinuationCanary 0x60000097c9c0> [48]

STACK OF 1 INSTANCE OF 'ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>>>':
18  libsystem_pthread.dylib               0x19d069d28 start_wqthread + 8
17  libsystem_pthread.dylib               0x19d06afd0 _pthread_wqthread + 228
16  libdispatch.dylib                     0x19ced06b8 _dispatch_worker_thread2 + 156
15  libdispatch.dylib                     0x19cecfea8 _dispatch_root_queue_drain + 392
14  libswift_Concurrency.dylib            0x2651dc470 swift_job_runImpl(swift::Job*, swift::ExecutorRef) + 72
13  libswift_Concurrency.dylib            0x2651db2e0 swift::runJobInEstablishedExecutorContext(swift::Job*) + 436
12  AsyncExtensionsPackageTests           0x1023971dc back deployment fallback for withCheckedContinuation<A>(isolation:function:_:) + 144  /<compiler-generated>:0
11  AsyncExtensionsPackageTests           0x102397350 closure #1 in withCheckedContinuation<A>(isolation:function:_:) + 220  /<compiler-generated>:0
10  AsyncExtensionsPackageTests           0x102391564 closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 708  AsyncBufferedChannel.swift:160
9   AsyncExtensionsPackageTests           0x10240d2c4 ManagedCriticalState.withCriticalRegion<A>(_:) + 188  Locking.swift:136
8   AsyncExtensionsPackageTests           0x102344a50 $ss13ManagedBufferCsRi__rlE25withUnsafeMutablePointersyqd_0_qd_0_SpyxG_Spyq_Gtqd__YKXEqd__YKs5ErrorRd__Ri_d_0_r0_lF + 172  /<compiler-generated>:0
7   AsyncExtensionsPackageTests           0x10240d430 partial apply for closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 52  /<compiler-generated>:0
6   AsyncExtensionsPackageTests           0x10240d394 closure #1 in ManagedCriticalState.withCriticalRegion<A>(_:) + 124  Locking.swift:139
5   AsyncExtensionsPackageTests           0x1023973a4 partial apply for closure #1 in closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 44  /<compiler-generated>:0
4   AsyncExtensionsPackageTests           0x102391ae4 closure #1 in closure #1 in closure #1 in AsyncBufferedChannel.next(onSuspend:) + 976  AsyncBufferedChannel.swift:166
3   libswiftCore.dylib                    0x1acd63c10 _allocateUninitializedArray<A>(_:) + 144
2   libswiftCore.dylib                    0x1ad0e27b8 swift_allocObject + 64
1   libswiftCore.dylib                    0x1ad0e24cc swift_slowAlloc + 36
0   libsystem_malloc.dylib                0x19cea8adc _malloc_zone_malloc_instrumented_or_legacy + 264 
====
    2 (96 bytes) ROOT LEAK: <Swift._ContiguousArrayStorage<AsyncExtensions.AsyncBufferedChannel<Swift.Int>.Awaiting<>> 0x600000959ad0> [48]
       1 (48 bytes) <CheckedContinuationCanary 0x600000959aa0> [48]

BTW, I've made a few other fixes on my branch to use the newer async test expectations API, use atomics, etc.

[lhoward]

lhoward/AsyncExtensions@dabc228 seems to fix this (using Array instead of OrderedSet for awaitings). Why, I don't know.

[lhoward]

This diff unfortunately won't cleanly apply as I've fixed a bunch of other issues (e.g. using the new withTaskCancellationHandler signature). But if you are amenable to merging I will prepare a PR.

[jamieQ]

i think the reported leaks may actually be due to the async iteration of the subjects never terminating, and so if there are Tasks somewhere awaiting them, the Task and its resources will effectively be leaked. for instance, this pattern is problematic:

do {
  let subject = AsyncCurrentValueSubject<Int>(0)
  Task {
    for await i in subject {
      print("\(i)")
    }
  }
}
// after this point, one can no longer cancel the Task nor terminate the subject's async iteration

you mentioned that the test suite seems to leak, and i noticed there are tests such as this one, which have this pattern that seems to be leaking Tasks, since they neither cancel the Tasks, nor end the async iteration.

[lhoward]

Yes, this could be it - I've gone around in circles this week looking for the leak - but I am careful to remove the reference to the task on cancellation.

My code below:

https://github.com/PADL/FlutterSwift/blob/main/Sources/FlutterSwift/Channel/FlutterEventChannel.swift#L165

[lhoward]

Also I knew leak detection isn't always completely deterministic but surely I'd still see the leak after switching to Array. I'll check if the tests are still leaking with my workaround.