tryFlushOutbox is not thread-safe

[jameshfisher]

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

[outbox removeObjectAtIndex:0];

... 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.

[PatrykKaczmarek]

Also it is easy to reproduce with following steps:

1. Initialize and configure PTPusher
2. implement registerWithDeviceToken:
3. move app to background.
4. make app active again.

[lukeredpath]

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.

[robinschaaf]

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) 👎

[devontivona]

Also having this issue. Is the best work-around a dispatch_after?

[ruimagalhaes]

Having this issue on the push-notifications branch. Did you found any workaround for this?

[robinschaaf]

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.

[b8ne]

@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.

[robinschaaf]

@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!

[b8ne]

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.