kean / Nuke

Image loading system
https://kean.blog/nuke
MIT License
8.06k stars 527 forks source link

Crash TaskFetchOriginalImageData on data.append #738

Open lukluca opened 9 months ago

lukluca commented 9 months ago

Hi!

I just noticed that there is a crash on TaskFetchOriginalImageData.

this is the stack trace

Signal: SIGTRAP EXC_BREAKPOINT (SIGTRAP): EXC_BREAKPOINT (SIGTRAP)

libswiftCore.dylib assertionFailure(:_:file:line:flags:) + 248 libswiftCore.dylib assertionFailure(:_:file:line:flags:) + 248 Foundation __DataStorage.replaceBytes(in:with:length:) + 2232 Foundation Data.InlineSlice.append(contentsOf:) + 100 Foundation Data.Representation.append(contentsOf:) + 356 Foundation 0x1a35d2000 + 3496304 Foundation Data.append(:) + 148 Circle TaskFetchOriginalImageData.dataTask(didReceiveData:response:) (TaskFetchOriginalImageData.swift:115) Circle partial apply for closure #1 in closure #1 in TaskFetchOriginalImageData.loadData(urlRequest:finish:) (TaskFetchOriginalImageData.swift:77) Circle thunk for @escaping @callee_guaranteed @Sendable () -> () (:0) libdispatch.dylib 0x1aff91000 + 407464 libdispatch.dylib 0x1aff91000 + 411520 libdispatch.dylib 0x1aff91000 + 259836 libdispatch.dylib 0x1aff91000 + 262576 libdispatch.dylib 0x1aff91000 + 302868 libsystem_pthread.dylib _pthread_wqthread + 284

On iOS 16

Nuke version 12.1.6

Seems that the affected line is inside this func

private func dataTask(didReceiveData chunk: Data, response: URLResponse)

of TaskFetchOriginalImageData, while appending data!

Unfortunately I am not able to replicate the crash, It happens in production few few times!

Thanks, Luca

kean commented 7 months ago

Hey, are you still able to reproduce this issue with the latest version?

larryonoff commented 6 months ago

I have similar crash, 5 crash events affecting 4 users last 30 days.

iOS: 15, 16 (not iOS 17) Nuke: 12.4.0

Crashed: com.github.kean.Nuke.ImagePipeline
0  Foundation                     0x359758 __DataStorage.init(bytes:length:copy:deallocator:offset:) + 368
1  Foundation                     0x35c39c Data.InlineSlice.ensureUniqueReference() + 160
2  Foundation                     0x35d55c Data.InlineSlice.append(contentsOf:) + 32
3  Foundation                     0x362444 Data._Representation.append(contentsOf:) + 356
4  Foundation                     0x355970 specialized __DataStorage.withUnsafeBytes<A>(in:apply:) + 92
5  Foundation                     0x365ee0 Data.append(_:) + 148
6  BEAT                           0x60fabc TaskFetchOriginalImageData.dataTask(didReceiveData:response:) + 115 (TaskFetchOriginalImageData.swift:115)
7  BEAT                           0x6112f8 partial apply for closure #1 in closure #1 in TaskFetchOriginalImageData.loadData(urlRequest:finish:) + 77 (TaskFetchOriginalImageData.swift:77)
8  BEAT                           0x5efcbc thunk for @escaping @callee_guaranteed @Sendable () -> () + 4347624636 (<compiler-generated>:4347624636)
9  libdispatch.dylib              0x637a8 _dispatch_call_block_and_release + 24
10 libdispatch.dylib              0x64780 _dispatch_client_callout + 16
11 libdispatch.dylib              0x3f6fc _dispatch_lane_serial_drain$VARIANT$armv81 + 600
12 libdispatch.dylib              0x401b0 _dispatch_lane_invoke$VARIANT$armv81 + 380
13 libdispatch.dylib              0x49f14 _dispatch_workloop_worker_thread + 608
14 libsystem_pthread.dylib        0x1bd0 _pthread_wqthread + 284
15 libsystem_pthread.dylib        0x1720 start_wqthread + 8
kean commented 5 months ago

Related: https://github.com/kean/Nuke/issues/572.

Hey, @larryonoff, do you happen to have any additional information around this crash? The size of the data? The crash message? Due to how rare these crashes seem to be, I suspect it's a memory issue – hitting a hard memory limit? If it is a memory issue, I wonder if I could add some defensive code to limit the size of expected data.

larryonoff commented 4 months ago

@kean I have only crash from Crashlytics. See it below.

Crashed: com.github.kean.Nuke.ImagePipeline
0  libswiftFoundation.dylib       0xfe768 <redacted> + 344
1  libswiftFoundation.dylib       0xab68 $s10Foundation4DataV11InlineSliceV21ensureUniqueReferenceyyF + 152
2  libswiftFoundation.dylib       0x12fb0 $s10Foundation4DataV11InlineSliceV6append10contentsOfySW_tF + 36
3  libswiftFoundation.dylib       0x13d5c $s10Foundation4DataV15_RepresentationO6append10contentsOfySW_tF + 568
4  BEAT                           0x7b929c TaskFetchOriginalImageData.dataTask(didReceiveData:response:) + 115 (TaskFetchOriginalImageData.swift:115)
5  BEAT                           0x7baad8 partial apply for closure #1 in closure #1 in TaskFetchOriginalImageData.loadData(urlRequest:finish:) + 77 (TaskFetchOriginalImageData.swift:77)
6  BEAT                           0x79949c thunk for @escaping @callee_guaranteed @Sendable () -> () + 4350137500 (<compiler-generated>:4350137500)
7  libdispatch.dylib              0x63094 <redacted> + 24
8  libdispatch.dylib              0x64094 <redacted> + 16
9  libdispatch.dylib              0xa73c <redacted> + 644
10 libdispatch.dylib              0xb1f4 <redacted> + 408
11 libdispatch.dylib              0x14ec8 <redacted> + 632
12 libsystem_pthread.dylib        0x1e00 _pthread_wqthread + 284
13 libsystem_pthread.dylib        0x192c start_wqthread + 8
Screenshot 2024-04-25 at 11 41 43

I would like to add that we had another similar issue (not with Nuke), when downloading video files incrementally. Our fix was checking the length of Data to get it contents. See the sample below.

func x() {
          let lowerBound = dataRequest.requestedOffset
          let upperBound = lowerBound + Int64(dataRequest.requestedLength)

          dataRequest.respond(with: data[safe: lowerBound..<upperBound])
}

private extension Data {
  subscript(
    safe range: Range<Int64>
  ) -> Data {
    guard !isEmpty else {
      return Data()
    }

    let safeRange = range.clamped(to: 0..<Int64(count))
    guard !safeRange.isEmpty else {
      return Data()
    }

    return self[safeRange]
  }
}