search

BoltsFramework / Bolts-ObjC

Bolts is a collection of low-level libraries designed to make developing mobile apps easier.
Other
5.65k stars 578 forks source link

Implement sync_ barrier based concurrency management #303

Closed flovilmart closed 5 years ago

flovilmart commented 7 years ago

Addresses issue #302 Also implements BoltsSwift resource protection through a queue and barrier_sync

cc @nlutsenko as you were marked in the TODO.

nlutsenko commented 7 years ago

@flovilmart, this is a good approach, though I would recommend looking at how we dealt with task state in Bolts-Swift and potentially porting it over here.

I will give it a tad more time to review, since it’s crucial to make sure that we’ll never get a deadlock, where we might have cases not covered by tests right now.

Also cc @richardjrossiii

flovilmart commented 7 years ago

Yeah, I encountered a few deadlocks while developing it. I’ll check out the Swift implementation for the state management, and improve.

flovilmart commented 7 years ago

@nlutsenko I introduced the BFTaskState, not sure if that's what you had in mind.

flovilmart commented 7 years ago

@nlutsenko We'll release our future QA version with this custom build, it all look good so far, no more thread sanitizer alert, no deadlocks.

flovilmart commented 7 years ago

@richardjrossiii I believe I addressed your suggestions let me know if there's anything else. I know this is a tricky part with concurrency and I'm a bit rusted on those problematics.

flovilmart commented 7 years ago

@richardjrossiii @nlutsenko the Parse SDK is all fine with that version and the thread sanitizer is happy for what I can tell. See: https://travis-ci.org/parse-community/Parse-SDK-iOS-OSX/builds/273015827

https://github.com/parse-community/Parse-SDK-iOS-OSX/pull/1185

flovilmart commented 6 years ago

@richardjrossiii @nlutsenko a quick ping on that one, I'm open to re-visiting another option that would let Xcode not warn about potential unsafe memory accesses.

flovilmart commented 6 years ago

@nlutsenko ping?

flovilmart commented 6 years ago

@nlutsenko any time for the review?

tealshift commented 6 years ago

I'm a bit out of my depth here. Can anyone can confirm this issue has potential to cause instability? I.e. would solving the problem actually make Bolts more stable, or is this mainly to silence Xcode's thread sanitizer tool? @nlutsenko @flovilmart