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

Call track async #18

Merged
merged 3 commits into from Jul 11, 2017
Merged

Call track async #18

merged 3 commits into from Jul 11, 2017

Conversation

ladanazita
Copy link
Contributor

@ladanazita ladanazita commented Jun 29, 2017

Fixes #17

All public API calls to the Taplytics SDK are dispatched synchronously:

#define dispatch_main_sync_safe(block)\
if ([NSThread isMainThread]) {\
    block();\
} else {\
    dispatch_sync(dispatch_get_main_queue(), block);\
}

This is causing deadlock issues with our analytics-ios SDK on key lifecycle events which must be handled synchronously. To resolve this, we will call the Taplytics SDK asynchronously.

});
}
}

- (void)reset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ladanazita

This will presumably need it as well, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes good catch, will add

@ladanazita
Copy link
Contributor Author

@VicV How does this look now with the requested changes? Thanks!

@VicV
Copy link
Contributor

VicV commented Jul 7, 2017

Hey @ladanazita, it works well for me, but I'm waiting for @bbernberg to confirm this works sometime next week.

@VicV
Copy link
Contributor

VicV commented Jul 10, 2017

@ladanazita looks like everything is working for the client. Good to go! 👍

Thanks for the quick response on this. Really appreciate it.

Copy link
Contributor

@f2prateek f2prateek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. DRY would be nice but not required atm.

@VicV
Copy link
Contributor

VicV commented Jul 11, 2017

Awesome. @ladanazita any idea when this will be in prod and good to go?

@ladanazita
Copy link
Contributor Author

@VicV I apologize for the delay, I was out of office yesterday. I can ship this today.

@ladanazita ladanazita merged commit 868f563 into master Jul 11, 2017
@ladanazita ladanazita deleted the fix/deadlock branch July 11, 2017 17:18
@ladanazita
Copy link
Contributor Author

@VicV this is on 1.1.2 now, thanks for the help!

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.

Call Taplytics async on track calls
3 participants