search

iZettle / Flow

Flow is a Swift library for working with asynchronous flows and life cycles
MIT License
232 stars 14 forks source link

Warnings: Initialization of 'PThreadMutex' results in a dangling pointer #110

Open nataliq-pp opened 3 years ago

nataliq-pp commented 3 years ago

Expected Behavior

No warnings produced by the framework

Current Behavior

13 compiler warnings

Steps to Reproduce

Always reproducible when building.

Context

Tried to address it here: https://github.com/iZettle/Flow/pull/104 @digitaliz did further investigation - Daniel please post info here

Failure Logs

Xcode shows: Initialization of 'PThreadMutex' (aka 'UnsafeMutablePointer<_opaque_pthread_mutex_t>') results in a dangling pointer which we started seeing recently.

and the following suggestions

1. Implicit argument conversion from 'pthread_mutex_t' (aka '_opaque_pthread_mutex_t') to 'UnsafeMutablePointer<pthread_mutex_t>' (aka 'UnsafeMutablePointer<_opaque_pthread_mutex_t>') produces a pointer valid only for the duration of the call to 'init(_:)'

2. Use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope
digitaliz commented 3 years ago

I'll give my assessment here on why&how we should fix this and also some background for what I've read/watched to come up with my reasoning.

An explanation of what a dangling pointer means is that the compiler can't guarantee that the pointer is still pointing to valid memory, leading to undefined behaviour. I think it was an oversight of Apple when they introduced the Unsafe* API to allow for direct initialization of these types as there is no way to guarantee the lifetime of the memory the pointer points to for any longer than the duration of the initialization call. Ideally this warning should be an error, but I think it's not because Apple had already committed to source compatibility.

To be able to guarantee pointer lifetimes Apple introduced the withUnsafe... methods and functions. If we use those the compiler will guarantee that the pointer is valid for the duration of the passed in closure, but it's unsafe to return the pointer from the closure and let it escape.

Since I last researched this after watching the WWDC2020 on Unsafe Swift & Pointer management I've now found this post by The Eskimo https://developer.apple.com/forums/thread/674633 much more clearly going into the details of the warning. So I recommend reading that and also watching the WWDC2020 videos, they are not that long and you may be able to watch them at 1.5x because they are pretty slow :)

Now, for the actual use inside Flow I don't have a good enough mental model of all this to understand if this issue triggers unwanted undefined behaviour (We use Flow in Go app successfully obviously), but I think we should fix it the warning if for no other reason than I think it implicitly means that Apple is free to break our code at any time.

I will submit a PR to fix the warnings but since it's also involving pthread_mutex_ts is has unearthed some other concerns, but we can address those on the PR.