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

Data race in ADJAttributionHandler, paused property #303

Closed
mindbrix opened this issue Aug 16, 2017 · 7 comments
Closed

Data race in ADJAttributionHandler, paused property #303

mindbrix opened this issue Aug 16, 2017 · 7 comments

Comments

@mindbrix
Copy link

In summary, the ADJAttributionHandler paused property is being written on one thread and written on another. The fix is to make the property atomic.

@property (atomic, assign) BOOL paused;

TSAN dump attached.

WARNING: ThreadSanitizer: data race (pid=27481)
  Read of size 1 at 0x7d1000052448 by thread T9:
    #0 -[ADJAttributionHandler paused] <null> (Adjust:x86_64+0x2fd8c)
    #1 -[ADJAttributionHandler requestAttributionI:] <null> (Adjust:x86_64+0x2e1b3)
    #2 __116-[ADJAttributionHandler initWithActivityHandler:withAttributionPackage:startsSending:hasAttributionChangedDelegate:]_block_invoke <null> (Adjust:x86_64+0x2c757)
    #3 __37-[ADJTimerOnce initBlock:queue:name:]_block_invoke <null> (Adjust:x86_64+0x60685)
    #4 __tsan::dispatch_callback_wrap(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6057a)
    #5 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x2b05b)

  Previous write of size 1 at 0x7d1000052448 by thread T5:
    #0 -[ADJAttributionHandler setPaused:] <null> (Adjust:x86_64+0x2fe27)
    #1 -[ADJAttributionHandler resumeSending] <null> (Adjust:x86_64+0x2d238)
    #2 -[ADJActivityHandler resumeSendingI:] <null> (Adjust:x86_64+0x17e4b)
    #3 -[ADJActivityHandler updateHandlersStatusAndSendI:] <null> (Adjust:x86_64+0x1785f)
    #4 -[ADJActivityHandler startI:] <null> (Adjust:x86_64+0xd213)
    #5 __48-[ADJActivityHandler applicationDidBecomeActive]_block_invoke <null> (Adjust:x86_64+0x3f7d)
    #6 __42+[ADJUtil launchInQueue:selfInject:block:]_block_invoke <null> (Adjust:x86_64+0x735a0)
    #7 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
    #8 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x2b05b)

  Location is heap block of size 56 at 0x7d1000052440 allocated by thread T5:
    #0 calloc <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x44222)
    #1 class_createInstance <null> (libobjc.A.dylib:x86_64+0xfba0)
    #2 +[ADJAdjustFactory attributionHandlerForActivityHandler:withAttributionPackage:startsSending:hasAttributionChangedDelegate:] <null> (Adjust:x86_64+0x26fcc)
    #3 -[ADJActivityHandler initI:sessionParametersActionsArray:] <null> (Adjust:x86_64+0xc7bf)
    #4 __67-[ADJActivityHandler initWithConfig:sessionParametersActionsArray:]_block_invoke <null> (Adjust:x86_64+0x3a79)
    #5 __42+[ADJUtil launchInQueue:selfInject:block:]_block_invoke <null> (Adjust:x86_64+0x735a0)
    #6 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x6043b)
    #7 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x2b05b)
@uerceg
Copy link
Contributor

uerceg commented Aug 16, 2017

@mindbrix

Thanks for reporting this. Will be checked and most probably become part of our upcoming iOS SDK update.

Will keep you posted in here.

Cheers

@uerceg
Copy link
Contributor

uerceg commented Aug 16, 2017

@mindbrix

Did you see any particular issue like crash caused by this? Or are you just getting this reported by the tool? Also, can you give us an info how to reproduce this what you are getting?

Thanks in advance.

@mindbrix
Copy link
Author

No crash, as the race condition is only on an Obj-C BOOL, stored as a single byte. TSAN error only.

The issue is caused by ADJUtil::launchInQueue:selfInject:block: being called on a different thread from the one ultimately used when ADJTimerOnce::timerWithBlock:queue:name: fires. Simply calling the former from the main queue should trigger TSAN when a timer fires as the blocks are dispatched onto internalQueue.

@uerceg
Copy link
Contributor

uerceg commented Aug 24, 2017

@mindbrix Quick update on this one - we have added it to our upcoming release. Once live, will update you in here.

@mindbrix
Copy link
Author

Thanks for responding so quickly ;-)

@sspitzer
Copy link
Contributor

sspitzer commented Sep 7, 2017

thanks for reporting this @mindbrix

@uerceg uerceg mentioned this issue Dec 12, 2017
@uerceg
Copy link
Contributor

uerceg commented Dec 13, 2017

@mindbrix

Thanks one more time for reporting this, is now became part of iOS SDK v4.12.0 (https://github.com/adjust/ios_sdk/releases/tag/v4.12.0).

Cheers. 🍺

@uerceg uerceg closed this as completed Dec 13, 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

No branches or pull requests

3 participants