apple / swift-collections

Commonly used data structures for Swift
Apache License 2.0
3.78k stars 297 forks source link

Possible memory leak in OrderedSet #437

Open lhoward opened 2 days ago

lhoward commented 2 days ago

I spent a bit of time trying to find a memory leak in AsyncExtension's AsyncBufferedChannel, and in the end it turned out to be fixed by replacing its use of OrderedSet with Array. Why this fixes the leak I am still investigating.

Information

I haven't tried with main, but there are no differences in Sources/OrderedCollections/OrderedSet between 1.1.4 and main.

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]
      ----
lhoward commented 2 days ago

Original source for AsyncBufferedChannel.

My branch which uses swift-atomics, new withTaskCancellationHandler() method signature, consolidates some locking and also fixes bug (note the leak occurred before making any of the first three fixes).

Ref sideeffect-io/AsyncExtensions#43