tryFlushOutbox is not thread-safe
Opened this issue · 9 comments
As reported by a user in support (thanks!):
Steps to reproduce
Call some combination of registerWithDeviceToken
, subscribe
, and unsubscribe
concurrently
Expected behavior
Eventually, all the subscribe/unsubscribe calls are processed in the order in which they were called
Actual behavior
This line ...
libPusher/Library/PTNativePusher.m
Line 123 in 436268e
... crashes with the error:
*** Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[__NSArrayM removeObjectAtIndex:]: index 0 beyond bounds for empty array'
Reason
(Hypothesized, not reproduced)
tryFlushOutbox
is not threadsafe. Between lookup object at index 0, and then removing the object at index 0, things may have changed. This may happen if tryFlushOutbox
is called multiple times: they will interfere with each other.
Also it is easy to reproduce with following steps:
- Initialize and configure
PTPusher
- implement
registerWithDeviceToken:
- move app to background.
- make app active again.
I have to admit, I'm not really happy with PTNativePusher, I feel like its a bit of a rush-job, but I haven't had time to work on the library myself recently.
I intend to revisit this when I get the chance.
Also having this issue, if I attempt to subscribe to more than one interest in succession (I've coded a non-ideal workaround using timeouts for now) 👎
Also having this issue. Is the best work-around a dispatch_after
?
Having this issue on the push-notifications
branch. Did you found any workaround for this?
For when I had to perform subscriptions in batch, I ended up having to use a promise and then once resolved, set a timeout of at least 100ms before firing the next one. According to their support we're supposed to be using PusherSwift instead (even though their documentation directs us to libPusher). However it's a swift library and messes up my cocoapods to have a mix of objective-c and swift, so I haven't had the time yet to devote to it.
@robinschaaf any chance you can share your code for this? both js and objective c? I have setup a bridged module but am experiencing the same issue. Im very new to the objective c side so any help would be appreciated.
@b8ne sure! I'm also a newbie to objective-c, so I'm not sure that this is the right way, but it seems to work.
Here is the RN (37) code with the promises and timeout (I'm also using redux, thunk and ramda's reduce here but you should get the idea):
export function syncInterestSubscriptions() {
return (dispatch, getState) => {
PROJECTS.reduce((promise, projectID) => {
return promise.then(() => {
return dispatch(updateInterestSubscription(projectID, true))
})
}, Promise.resolve())
}
}
export function updateInterestSubscription(interest, subscribed) {
var NotificationSettings = NativeModules.NotificationSettings
return () => {
return new Promise((resolve) => {
NotificationSettings.setInterestSubscription(interest, subscribed).then((message) => {
setTimeout(()=> {
return resolve(message)
}, 500)
})
})
}
}
Then this is all of the pertinent objective-c code, that allows you to refer to the pusher via the NativeModules
AppDelegate.h:
#import <UIKit/UIKit.h>
#import <Pusher/Pusher.h>
#import <UserNotifications/UserNotifications.h>
@interface AppDelegate : UIResponder <UIApplicationDelegate, PTPusherDelegate, UNUserNotificationCenterDelegate>
@property (nonatomic, strong) UIWindow *window;
@property (nonatomic, strong) PTPusher *pusher;
@end
AppDelegate.m:
...
- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
...
self.pusher = [PTPusher pusherWithKey:@"xxxxx" delegate:self encrypted:YES];
...
}
- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken {
[[[self pusher] nativePusher] registerWithDeviceToken:deviceToken];
[[[self pusher] nativePusher] subscribe:@"general"];
}
NotificationSettings.h:
#import "RCTBridgeModule.h"
#import "AppDelegate.h"
#import <Pusher/Pusher.h>
@interface NotificationSettings : NSObject <RCTBridgeModule, PTPusherDelegate>
@property (nonatomic, strong) PTPusher *pusher;
@end
NotificationSettings.m:
#import "NotificationSettings.h"
#import "RCTLog.h"
#import <Pusher/Pusher.h>
@implementation NotificationSettings
RCT_EXPORT_MODULE();
RCT_EXPORT_METHOD(setInterestSubscription:(NSString *)interest
subscribed:(BOOL *)subscribed
resolver:(RCTPromiseResolveBlock)resolve
rejecter:(RCTPromiseRejectBlock)reject)
{
AppDelegate *appDelegate = (AppDelegate*)[[UIApplication sharedApplication] delegate];
self.pusher = appDelegate.pusher;
if (subscribed) {
[[[self pusher] nativePusher] subscribe:interest];
} else {
[[[self pusher] nativePusher] unsubscribe:interest];
}
resolve(@"Interest subscription set");
}
@end
Hope that helps!
hey @robinschaaf thanks heaps for sharing. Just after I posted to you I found this. Obviously its not merged, so not sure if it will be legit, but ive tested it and it works. Now my code is like this
NotificationManager.m
#import "NotificationManager.h"
#import "AppDelegate.h"
#import "RCTLog.h"
@implementation NotificationManager
- (dispatch_queue_t)methodQueue
{
return dispatch_queue_create("com.mirk.Maskot.AsyncNotificationsStorageQueue", DISPATCH_QUEUE_SERIAL);
}
RCT_EXPORT_MODULE();
RCT_EXPORT_METHOD(subscribe:(NSString *)channel)
{
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
RCTLogInfo(@"Subscribing to: %@", channel);
AppDelegate *appDelegate = (AppDelegate *)[[UIApplication sharedApplication] delegate];
[appDelegate subscribeToChannel:(NSString *)channel];
});
}
RCT_EXPORT_METHOD(unsubscribe:(NSString *)channel)
{
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
RCTLogInfo(@"Unsubscribing from: %@", channel);
AppDelegate *appDelegate = (AppDelegate *)[[UIApplication sharedApplication] delegate];
[appDelegate unsubscribeChannel:(NSString *)channel];
});
}
@end
AppDelegate.m
- (void)subscribeToChannel:(NSString *)channel
{
[[[self pusher] nativePusher] subscribe:(NSString *)channel];
}
- (void)unsubscribeChannel:(NSString *)channel
{
[[[self pusher] nativePusher] unsubscribe:(NSString *)channel];
}
@end
AppDelegate.h
-(void)subscribeToChannel:(NSString *)channel;
-(void)unsubscribeChannel:(NSString *)channel;
@end
I havent looked too far into objective c multithreading, but im guessing i could probably do without the async stuff now, but its working so im just going to leave it.