close
Skip to content

Prevent race condition for lookups#2

Merged
ashfurrow merged 1 commit into
RxSwiftCommunity:masterfrom
lipka:master
Nov 16, 2015
Merged

Prevent race condition for lookups#2
ashfurrow merged 1 commit into
RxSwiftCommunity:masterfrom
lipka:master

Conversation

@lipka
Copy link
Copy Markdown
Contributor

@lipka lipka commented Nov 16, 2015

There is a possible scenario for a race condition when doing lookups in the existing code from multiple threads.

  1. T1 tries to do a lookup, acquires lock, but doesn't find anything
  2. T2 tries to do a lookup, fails to acquire lock, waits while T1 releases lock
  3. T1 releases lock and leaves the critical section without a lookup
  4. T2 acquires the lock, tries to do a lookup, finds nothing, leaves critical section without a lookup
  5. T1 finally creates a disposable bag
  6. T2 creates a disposable bag too, overwrites disposable bag from T1

The suggested solution does both the lookup and assignment in the same critical section.

(Sorry about the force unwrapped optional!)

@ashfurrow
Copy link
Copy Markdown
Member

This is fantastic @lipka 💯 Thanks for the pull request! I'll merge, add a changelog entry, and submit a new release to CocoaPods Trunk 🎉

ashfurrow added a commit that referenced this pull request Nov 16, 2015
Prevent race condition for lookups
@ashfurrow ashfurrow merged commit c484072 into RxSwiftCommunity:master Nov 16, 2015
@ashfurrow
Copy link
Copy Markdown
Member

I agree about the sad necessity of the implicitly unwrapped optional – could make the doLocked function return a generic or something, but using an implicitly unwrapped optional is close enough 😸

I've pushed a new release up to trunk: https://raw.githubusercontent.com/CocoaPods/Specs/89f45b897166a93e5ecbb427dffdf875eef85ad9/Specs/NSObject+Rx/1.1.0/NSObject+Rx.podspec.json

Also, I added you as a collaborator on the repo – may ask for your input on some pull requests if that's ok.

Thanks again!

Maxim-Inv added a commit to Maxim-Inv/NSObject-Rx that referenced this pull request Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants