Fix HTTP Race Condition #372
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
• Fixes a relatively rare issue that would be encountered if the SDK attempted to make concurrent HTTP requests during an invalid situation, such as after the user has revoked GDPR consent • The SDK was previously using a semaphore to wait for a batch of HTTP requests to finish. • However, if the SDK immediately fails the request (synchronously), the request would fail before the SDK started waiting for the request to finish, resulting in a permanently blocked queue. • Switched to using a dispatch_group which would handle this situation better. Also added a missing GDPR consent check to the onFocus methods
jkasten2
requested changes
May 18, 2018
| if (requestData) { | ||
| NSString *json = [[NSString alloc] initWithData:requestData encoding:NSUTF8StringEncoding]; | ||
|
|
||
| NSLog(@"Executing request %@ with json: %@", NSStringFromClass([request class]), json); |
Member
There was a problem hiding this comment.
Our OneSignal logging method should be used instead of NSLog
• Removes a debug statement from the previous commit
jkasten2
approved these changes
May 18, 2018
baptiste-nv
added a commit
to nventive/OneSignal-iOS-SDK
that referenced
this pull request
Jul 4, 2018
* Fix HTTP Race Condition (OneSignal#372) * Fix HTTP Concurrency • Fixes a relatively rare issue that would be encountered if the SDK attempted to make concurrent HTTP requests during an invalid situation, such as after the user has revoked GDPR consent • The SDK was previously using a semaphore to wait for a batch of HTTP requests to finish. • However, if the SDK immediately fails the request (synchronously), the request would fail before the SDK started waiting for the request to finish, resulting in a permanently blocked queue. • Switched to using a dispatch_group which would handle this situation better. Also added a missing GDPR consent check to the onFocus methods * Remove Debug Statement • Removes a debug statement from the previous commit * 2.8.5 Release • Resolves a rare concurrency issue with the SDK's HTTP client * Miscellaneous Fixes (OneSignal#370) * Miscellaneous Fixes • Fixes an issue where the download method for attachments wasn't correctly implemented with a double pointer. It accepted NSError as a parameter, but because this pointer gets copied, any new assignments will apply to the copied pointer, not the original. • Fixes an issue in setEmail() where the failure block was being called without verifying that it is nonnull, which could cause crashes. • Cleans up the notificationReceived() method, which created a string (actionSelected) but doesn't use it. * Fix Method Signature • A recent bug fix changed the signature of a helper method to synchronously download files • Fixes the method signature so the swizzled test override method works and doesn't crash the test * Update location tracking + add significant location changes monitoring (#1) * Add lat/long tags
baptiste-nv
added a commit
to nventive/OneSignal-iOS-SDK
that referenced
this pull request
Jul 11, 2018
* Fix HTTP Race Condition (OneSignal#372) * Fix HTTP Concurrency • Fixes a relatively rare issue that would be encountered if the SDK attempted to make concurrent HTTP requests during an invalid situation, such as after the user has revoked GDPR consent • The SDK was previously using a semaphore to wait for a batch of HTTP requests to finish. • However, if the SDK immediately fails the request (synchronously), the request would fail before the SDK started waiting for the request to finish, resulting in a permanently blocked queue. • Switched to using a dispatch_group which would handle this situation better. Also added a missing GDPR consent check to the onFocus methods * Remove Debug Statement • Removes a debug statement from the previous commit * 2.8.5 Release • Resolves a rare concurrency issue with the SDK's HTTP client * Miscellaneous Fixes (OneSignal#370) * Miscellaneous Fixes • Fixes an issue where the download method for attachments wasn't correctly implemented with a double pointer. It accepted NSError as a parameter, but because this pointer gets copied, any new assignments will apply to the copied pointer, not the original. • Fixes an issue in setEmail() where the failure block was being called without verifying that it is nonnull, which could cause crashes. • Cleans up the notificationReceived() method, which created a string (actionSelected) but doesn't use it. * Fix Method Signature • A recent bug fix changed the signature of a helper method to synchronously download files • Fixes the method signature so the swizzled test override method works and doesn't crash the test * Bundle ID Improvement (OneSignal#377) • In order to allow communication between the App Extension and the host/primary application, OneSignal SDK uses an app group to perform communication • For most developers, the name of the app group will simply be group.{your_bundle_id}.onesignal • The SDK can automatically retrieve this bundle ID, but previously, for the app extension service, this was broken because it was only retrieving the bundle ID for the OneSignalNotificationServiceExtension • This means the bundle ID's would not have matched and badge count would be inconsistent. This commit fixes the issue by using a method to retrieve the host app's bundle ID. * Update location tracking + add significant location changes monitoring (#1) * Disable fine location tracking when app is background (#3) * Ensure location tags are properly updated with no delay (#4) * Update location tracking method
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
• Fixes a relatively rare issue that would be encountered if the SDK attempted to make concurrent HTTP requests during an invalid situation, such as after the user has revoked GDPR consent, or if the SDK hasn't received the app ID yet.
• The SDK was previously using a semaphore to wait for a batch of HTTP requests to finish.
• However, if the SDK immediately fails the request (synchronously), the request would fail before the SDK started waiting for the request to finish, resulting in a permanently blocked queue.
• Switched to using a dispatch_group which would handle this situation better. Also added a missing GDPR consent check to the onFocus methods
This change is