Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3] [ios] transaction #215

Merged
merged 10 commits into from
Jan 26, 2017
Merged

[v3] [ios] transaction #215

merged 10 commits into from
Jan 26, 2017

Conversation

FrankSalad
Copy link

@FrankSalad FrankSalad commented Jan 6, 2017

@auser I hope this is similar to what you might've done for #78. It's a little bit brittle - it can leak if the beginTransaction call isn't followed up with a tryCommitTransaction. If you had a different implementation strategy in mind I'd like to know 🤸‍♂️

@Ehesp
Copy link
Contributor

Ehesp commented Jan 6, 2017

cc @Salakar @chrisbianca

@FrankSalad
Copy link
Author

Looking at the concurrency again, I don't understand why the 'dispatch_barrier_async' within the 'dispatch_async' call doesn't deadlock every time. Does GCD run the barrier block when it notices the outer block running 'dispatch_semaphore_wait'?

All the docs say barrier blocks do not execute while other blocks on the same queue are executing. Does waiting on a semaphore not count as executing on the queue?

I'm still kind of green when it comes to native code. @auser I'll add info in the docs asap. Thanks.

@auser
Copy link
Contributor

auser commented Jan 7, 2017

@FrankSalad lmk when you add to the docs. As far as the dispatch async, it does 'queue' it AFAIK, so I think that should work just fine

@FrankSalad
Copy link
Author

@Salakar
Copy link
Collaborator

Salakar commented Jan 10, 2017

@FrankSalad nice work, can we get this in on android also if anyone has time to do it?

cc @Ehesp @chrisbianca

@Salakar Salakar changed the title V3 transaction [v3] [ios] transaction Jan 12, 2017
@auser auser merged commit b395e26 into fullstackreact:v3 Jan 26, 2017
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.

4 participants