search

facebookarchive / pop

An extensible iOS and OS X animation library, useful for physics-based interactions.
Other
19.66k stars 2.88k forks source link

Replace OSSpinLock with os_unfair_lock_t/pthread_mutex_t #336

Closed LeoNatan closed 7 years ago

LeoNatan commented 7 years ago

OSSpinLock is considered harmful and unsafe for use on iOS and macOS (under 2015/2016 MacBook). Instead, you should use os_unfair_lock_t if FIFO order is not important, or pthread_mutex_t, which while less performant, is not prone to priority inversion issues.

nlutsenko commented 7 years ago

pthread_mutex_t is preferred in our case, but agreed that OSSpinLock is harmful, though I wouldn't imagine that we would suffer from priority inversion, since our usage of spin locks is very very narrow.

There has been an effort to improve the situation with #321, but dispatch_semaphore_t isn't the way we would want to take it. Me, and every else owner of pop would love to see the PR for this and we'll review and merge it in!

hfossli commented 7 years ago

Why do we need locks at all? CADisplayLink is the driver for all animations and is added to the main run loop, isn't it?

jhoughjr commented 7 years ago

My 2c, OSSpinLock is deprecated in iOS 10, and yields many warnings about it with a pod integration on iOS 10.

LeoNatan commented 7 years ago

@grp @piemonte This seems to have been resolved. Should I close the issue?

piemonte commented 7 years ago

@grp yep, thanks much ✌️