guillermomuntaner / Burritos

A collection of Swift Property Wrappers (formerly "Property Delegates")
MIT License
1.33k stars 43 forks source link

@Lazy* property wrappers are not thread safe #18

Open Larry-Gensch opened 4 years ago

Larry-Gensch commented 4 years ago

The @Lazy and @LazyConstant property wrappers do not appear to be thread safe. That is, if two threads attempt to hit the initial getter at the exact same time, they will both attempt to initialize the value. Running the initializer multiple times can have the adverse effect of executing the "expensive operation" multiple times, which is not a good thing.

Note: This only would happen if the @Lazy objects are accessed in multiple threads. For UIKit initializers, everything must be done on the main (synchronous) thread, and so we don't have this issue.

The "standard" solution for such a case is creating a concurrent queue, and doing the work within the getter in a queue.sync{} block, and (for the non-constant @Lazy implementation), putting the setter into an async{} .barrier to ensure the reads and writes are synchronized. This would require that a DispatchQueue be created for every @Lazy property, but this has the side-effect of possibly creating many, many GCD queues, which isn't needed for the UIKit exception I mentioned above. This might require maintaining a pool of thread objects, assigned to each @Lazy object, but we're now adding even more complexity to what should be a simple property wrapper.

So, my suggestion is to provide two additional property wrappers to have a total of four @Lazy style objects:

  1. @Lazy -- Current modifiable implementation
  2. @LazyConstant -- Current constant implementation
  3. @LazyQueue -- @Lazy + barrier queues
  4. @LazyQueueConstant -- @LazyConstant + synchronous queues

An even better implementation would allow the passing of an OptionSet to supply [.constant, .queue] to the initializer, but that makes the @Lazy(flags: [], wrappedValue: { ... } var foo quite complex to write.