-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Description
Environment
- Kingfisher version: 8.5.0
- Swift version: 5.5
- Xcode version: 16.2
- Build configuration: Release (-O)
Summary
KFImage
extension methods that mutate context
via a var image = self; image.context = ...; return image
pattern crash only in Release builds (does not reproduce in Debug). The crash appears tied to optimized builds and likely stems from unsafe in-place mutation of KFImage
/its context
, escaping closures + AnyView
type-erasure, or optimizer-induced undefined behavior.
Steps to reproduce (minimal)
- Add an extension on
KFImage
like the following and use it in a SwiftUI view:
extension KFImage {
public func kfPlaceholder<P: View>(@ViewBuilder _ content: @escaping (Progress) -> P) -> Self {
var image = self
image.context.placeholder = { progress in AnyView(content(progress)) }
return image
}
public func kfCancelOnDisappear(_ flag: Bool) -> Self {
var image = self
image.context.cancelOnDisappear = flag
return image
}
}
- Build the app using Release configuration (Optimize for Speed) and run on device or TestFlight.
- Use the modifiers above while loading images. The app crashes during view composition / image lifecycle.
Expected behavior
Modifiers should safely set placeholder/failure/cancel flags without crashing in Release builds.
Actual behavior
- Crash only in Release (optimized) builds.
- No crash in Debug builds.
- Removing the in-place mutation or delegating to the protocol-level API removes the crash.
Observed clues
- Crash is optimizer-dependent (Release-only).
- In-place mutation of
self.context
withvar image = self
is correlated with failure. - Escaping
ViewBuilder
-based closures andAnyView
used as placeholder are involved. - Stack traces vary; crash often occurs when view is created/destroyed or closure executes.
Analysis — plausible causes
-
Unsafe in-place mutation / copy-on-write aliasing
KFImage
is aView
(value type). Mutating a nestedcontext
that may be a reference type or has COW semantics can create aliasing or use-after-free under -O optimizations.
-
Escaping closures + type-erasure lifetime
- Storing an escaping closure that returns
AnyView
into internal context could violate lifetime assumptions when optimized.
- Storing an escaping closure that returns
-
Thread-safety / data race
- If
context
is shared across copies or mutated on different threads, timing differences in Release can trigger races.
- If
-
Compiler/optimizer bug
- Complex interactions between value mutation, escaping closures, and SwiftUI may expose an optimizer bug only visible in Release.
Suggested fix (safe, minimal)
Remove the mutating KFImage
helpers and provide non-mutating wrappers that delegate to the existing KFImageProtocol
modifiers or return some View
. This avoids in-place mutation of a value-type view.
Replace the problematic methods:
/// Sets a placeholder `View` that is displayed during the image loading, with a progress parameter as input.
///
/// - Parameter content: A view that represents the placeholder.
/// - Returns: A Kingfisher-compatible image view that includes the provided `content` as its placeholder.
public func placeholder<P: View>(@ViewBuilder _ content: @escaping (Progress) -> P) -> Self {
context.placeholder = { progress in
return AnyView(content(progress))
}
return self
}
/// Sets a placeholder `View` that is displayed during the image loading.
///
/// - Parameter content: A view that represents the placeholder.
/// - Returns: A Kingfisher-compatible image view that includes the provided `content` as its placeholder.
public func placeholder<P: View>(@ViewBuilder _ content: @escaping () -> P) -> Self {
placeholder { _ in content() }
}
/// Enables canceling the download task associated with `self` when the view disappears.
///
/// - Parameter flag: A boolean value indicating whether to cancel the task.
/// - Returns: A Kingfisher-compatible image view that cancels the download task when it disappears.
public func cancelOnDisappear(_ flag: Bool) -> Self {
context.cancelOnDisappear = flag
return self
}
With non-mutating wrappers that call the protocol-level API:
extension KFImageProtocol {
public func placeholder<P: View>(@ViewBuilder _ content: @escaping (Progress) -> P) -> some View {
// Delegate to KFImageProtocol's safe modifier
return self.placeholder { progress in AnyView(content(progress)) }
}
public func placeholder<P: View>(@ViewBuilder _ content: @escaping () -> P) -> some View {
return kfPlaceholder { _ in content() }
}
public func cancelOnDisappear(_ flag: Bool) -> some View {
return self.cancelOnDisappear(flag)
}
}
Rationale
- Avoids mutating nested state on a value-type
View
. - Returning
some View
avoids unsafeSelf
mutation semantics and plays better with optimizer/copy semantics. - Consolidates behavior to the protocol-level API which already exists.
Additional recommendations
- If
context
is a reference type, consider copy-on-write semantics or making mutation thread-safe. - Add a test that compiles and runs the relevant code path in Release optimization to detect regressions.
- Run Address/Thread/UB sanitizers in Release-like modes to reveal undefined behavior or data races.
- If possible, include crash logs / stack traces from Release runs to help triage.
Temporary workaround
Use the KFImageProtocol
modifiers directly (non-mutating), or remove var image = self
mutation. Keep placeholder/failure views supplied at call site via the protocol API.
Request
If maintainers want to triage further, please advise which debug artifacts are most helpful (symbolicated crash log, release-mode sample project). I can prepare a minimal reproducible sample or a PR implementing the non-mutating wrappers and tests.
Labels suggestion
bug
, release-only
, swift-optimizer
, help wanted