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

Race conditions related to PFEventuallyQueue, PFCommandCache #1175

Closed
QB3L opened this issue Aug 22, 2017 · 4 comments
Closed

Race conditions related to PFEventuallyQueue, PFCommandCache #1175

QB3L opened this issue Aug 22, 2017 · 4 comments

Comments

@QB3L
Copy link

QB3L commented Aug 22, 2017

Been seeing this a lot. Seems to be related to setting the connected state.

==================
WARNING: ThreadSanitizer: data race (pid=58906)
Write of size 1 at 0x7d1000012862 by thread T2 (mutexes: write M1031):
#0 -[BFTask setCompleted:] BFTask.m (Bolts:x86_64+0x104a7)
#1 -[BFTask trySetResult:] BFTask.m:302 (Bolts:x86_64+0xbc33)
#2 -[BFTaskCompletionSource setResult:] BFTaskCompletionSource.m:46 (Bolts:x86_64+0x10d07)
#3 __34-[PFEventuallyQueue setConnected:]_block_invoke PFEventuallyQueue.m:404 (Parse:x86_64+0x73040)
#4 __tsan::invoke_and_release_block(void*) :1206112 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
#5 _dispatch_client_callout :1206112 (libdispatch.dylib:x86_64+0x2b05b)

Previous read of size 1 at 0x7d1000012862 by thread T6 (mutexes: write M283582895120049968):
#0 -[BFTask waitUntilFinished] BFTask.m:528 (Bolts:x86_64+0xfbeb)
#1 -[BFTask(Private) waitForResult:withMainThreadWarning:] BFTask+Private.m:105 (Parse:x86_64+0x375c)
#2 -[BFTask(Private) waitForResult:] BFTask+Private.m:100 (Parse:x86_64+0x36ab)
#3 -[PFEventuallyQueue setConnected:] PFEventuallyQueue.m:414 (Parse:x86_64+0x72dfa)
#4 -[PFEventuallyQueue _startMonitoringNetworkReachability] PFEventuallyQueue.m:370 (Parse:x86_64+0x728d5)
#5 -[PFEventuallyQueue initWithDataSource:maxAttemptsCount:retryInterval:] PFEventuallyQueue.m:78 (Parse:x86_64+0x6c4b0)
#6 -[PFCommandCache initWithDataSource:coreDataSource:maxAttemptsCount:retryInterval:diskCachePath:diskCacheSize:] PFCommandCache.m:76 (Parse:x86_64+0x31d03)
#7 +[PFCommandCache newDefaultCommandCacheWithCommonDataSource:coreDataSource:cacheFolderPath:] PFCommandCache.m:60 (Parse:x86_64+0x31a48)
#8 -[ParseManager _newCommandCache] ParseManager.m:200 (Parse:x86_64+0xbeb3)
#9 __31-[ParseManager eventuallyQueue]_block_invoke ParseManager.m:172 (Parse:x86_64+0xb7fa)
#10 __tsan::invoke_and_release_block(void*) :1206112 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
#11 _dispatch_client_callout :1206112 (libdispatch.dylib:x86_64+0x2b05b)
#12 -[ParseManager eventuallyQueue] ParseManager.m:162 (Parse:x86_64+0xb3e1)
#13 __47-[ParseManager preloadDiskObjectsToMemoryAsync]_block_invoke ParseManager.m:448 (Parse:x86_64+0x13198)
#14 __37+[BFTask taskFromExecutor:withBlock:]_block_invoke BFTask.m:285 (Bolts:x86_64+0xb8b7)
#15 __55-[BFTask continueWithExecutor:block:cancellationToken:]_block_invoke BFTask.m:412 (Bolts:x86_64+0xda9a)
#16 __tsan::invoke_and_release_block(void*) :1206112 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
#17 _dispatch_client_callout :1206112 (libdispatch.dylib:x86_64+0x2b05b)

Location is heap block of size 64 at 0x7d1000012840 allocated by thread T6:
#0 calloc :1206144 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x44222)
#1 class_createInstance :1206144 (libobjc.A.dylib:x86_64+0xfba0)
#2 +[BFTaskCompletionSource taskCompletionSource] BFTaskCompletionSource.m:31 (Bolts:x86_64+0x1095a)
#3 -[PFEventuallyQueue setConnected:] PFEventuallyQueue.m:393 (Parse:x86_64+0x72b31)
#4 -[PFEventuallyQueue _startMonitoringNetworkReachability] PFEventuallyQueue.m:370 (Parse:x86_64+0x728d5)
#5 -[PFEventuallyQueue initWithDataSource:maxAttemptsCount:retryInterval:] PFEventuallyQueue.m:78 (Parse:x86_64+0x6c4b0)
#6 -[PFCommandCache initWithDataSource:coreDataSource:maxAttemptsCount:retryInterval:diskCachePath:diskCacheSize:] PFCommandCache.m:76 (Parse:x86_64+0x31d03)
#7 +[PFCommandCache newDefaultCommandCacheWithCommonDataSource:coreDataSource:cacheFolderPath:] PFCommandCache.m:60 (Parse:x86_64+0x31a48)
#8 -[ParseManager _newCommandCache] ParseManager.m:200 (Parse:x86_64+0xbeb3)
#9 __31-[ParseManager eventuallyQueue]_block_invoke ParseManager.m:172 (Parse:x86_64+0xb7fa)
#10 __tsan::invoke_and_release_block(void*) :1206144 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
#11 _dispatch_client_callout :1206144 (libdispatch.dylib:x86_64+0x2b05b)
#12 -[ParseManager eventuallyQueue] ParseManager.m:162 (Parse:x86_64+0xb3e1)
#13 __47-[ParseManager preloadDiskObjectsToMemoryAsync]_block_invoke ParseManager.m:448 (Parse:x86_64+0x13198)
#14 __37+[BFTask taskFromExecutor:withBlock:]_block_invoke BFTask.m:285 (Bolts:x86_64+0xb8b7)
#15 __55-[BFTask continueWithExecutor:block:cancellationToken:]_block_invoke BFTask.m:412 (Bolts:x86_64+0xda9a)
#16 __tsan::invoke_and_release_block(void*) :1206144 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
#17 _dispatch_client_callout :1206144 (libdispatch.dylib:x86_64+0x2b05b)

Mutex M1031 (0x7d180001fe78) created at:
#0 pthread_mutex_lock :1206000 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x34ace)
#1 recursive_mutex_tt::lock() :1206000 (libobjc.A.dylib:x86_64+0x10046)
#2 _dispatch_client_callout :1206000 (libdispatch.dylib:x86_64+0x2b05b)

Mutex M283582895120049968 is already destroyed.

Thread T2 (tid=1782772, running) created by thread T-1
[failed to restore the stack]

Thread T6 (tid=1782784, running) created by thread T-1
[failed to restore the stack]
Summary: ThreadSanitizer: data race BFTask.m in -[BFTask setCompleted:]

ThreadSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.

==================

@flovilmart
Copy link
Contributor

flovilmart commented Aug 22, 2017

Seems that the issue is in Bolts and not the Parse SDK. I ran the thread sanitizer and realized this:

Upon callback, setting the result on a particular thread

- (BOOL)trySetResult:(nullable id)result {
    @synchronized(self.lock) {
        if (self.completed) {
            return NO;
        }
        self.completed = YES; // <-- HERE
        _result = result;
        [self runContinuations];
        return YES;
    }
}

when a task is marked waitUntilFinished 

- (void)waitUntilFinished {
    if ([NSThread isMainThread]) {
        [self warnOperationOnMainThread];
    }

    @synchronized(self.lock) {
        if (self.completed) {
            return;
        }
        [self.condition lock];
    }
    // TODO: (nlutsenko) Restructure this to use Bolts-Swift thread access synchronization architecture
    // In the meantime, it's absolutely safe to get `_completed` aka an ivar, as long as it's a `BOOL` aka less than word size.
    while (!_completed) { // <- READ data from there.
        [self.condition wait];
    }
    [self.condition unlock];
}

Also there seems to be a note about it:

// TODO: (nlutsenko) Restructure this to use Bolts-Swift thread access synchronization architecture
// In the meantime, it's absolutely safe to get `_completed` aka an ivar, as long as it's a `BOOL` aka less than word size.

We see other data races in bolts related to the concurrency model employed, mostly read on BOOL completed that are concurrent and not on the same thread.

@QB3L
Copy link
Author

QB3L commented Aug 22, 2017

This is really good to know. I'll keep an eye for any other stuff. Should we close this until they fix it on their side then?

@flovilmart
Copy link
Contributor

We should probably open an issue and provide a fix :)

@stale
Copy link

stale bot commented Sep 19, 2018

This issue has been automatically marked as stale because it has not had recent activity. If you believe it should stay open, please let us know! As always, we encourage contributions, check out the Contributing Guide

@stale stale bot added the wontfix label Sep 19, 2018
@stale stale bot closed this as completed Sep 26, 2018
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

No branches or pull requests

2 participants