Sparkle AppUpdaterDelegate destruction not happening - causes a crash

[BobBrownConsulting]

I'm seeing an issue where dealloc is not being called which results in the Sparkle layer of code not cleaning up and then crashing after my main object is deleted.

Below shows the C++ class that is using QT QObject.

When the VersioningController is destroyed which happens during our application workflow after the user requests a version check and the version check is completed, the code crashes since the AppUpdaterDelegate never destroys properly.

Two possible thoughts as to what the problem is:

Thought 1:

My Obj-C is a little rusty and I didn't set up the code properly.

Or I'm not shutting down the AppUpdaterDelegate properly.

Thought 2:

The Sparkle layer (AppUpdaterDelegate) needs to be alive for the entire lifetime of the application. Though I worry the same issue with app shutdown might occur.

Any help debugging and resolving this issue is much appreciated!

class VersioningController : public QObject
{
    Q_OBJECT

public:
    explicit VersioningController();
    ~VersioningController() override;

#ifdef __OBJC__
    // Expose ObjC type only to updater_sparkle.mm. This allows ARC to properly track its lifetime.
    AppUpdaterDelegate *_updaterDelegate;
#else
    void *_updaterDelegate;
#endif
};

// An updater delegate class that mainly takes care of updating the check for updates menu item's state
// This class can also be used to implement other updater delegate methods
@interface AppUpdaterDelegate : NSObject <SPUUpdaterDelegate>

@property(nonatomic) SPUStandardUpdaterController *updaterController;

@end

@implementation AppUpdaterDelegate

- (void)observeCanCheckForUpdatesWithController:(VersioningController *)controller
{
    [_updaterController.updater addObserver:self
                                 forKeyPath:NSStringFromSelector(@selector(canCheckForUpdates))
                                    options:(NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew)
                                    context:controller];
    [_updaterController.updater addObserver:self
                                 forKeyPath:NSStringFromSelector(@selector(sessionInProgress))
                                    options:(NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew)
                                    context:controller];
}

- (void)updater:(SPUUpdater *)updater
    userDidMakeChoice:(SPUUserUpdateChoice)choice
            forUpdate:(SUAppcastItem *)updateItem
                state:(SPUUserUpdateState *)state
{
    if (choice == SPUUserUpdateChoiceDismiss)
    {
        sUserCanceled = true;
    }
}

- (void)observeValueForKeyPath:(NSString *)keyPath
                      ofObject:(id)object
                        change:(NSDictionary<NSKeyValueChangeKey, id> *)change
                       context:(void *)context
{    
    if (keyPath == nullptr)
    {
        return;
    }

    if (context == nullptr)
    {
        return;
    }

VersioningController *controller = (VersioningController *)context;

    if ([keyPath isEqualToString:NSStringFromSelector(@selector(canCheckForUpdates))])
    {
        if (_updaterController.updater.canCheckForUpdates)
        {
            return;
        }
    }
    else if ([keyPath isEqualToString:NSStringFromSelector(@selector(sessionInProgress))])
    {
        if (_updaterController.updater.sessionInProgress)
        {
            return;
        }
        else
        {
            controller->updateDone();
        }
    }
    else
    {
        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }
}

- (void)dealloc
{
    @autoreleasepool
    {
        [_updaterController.updater removeObserver:self forKeyPath:NSStringFromSelector(@selector(canCheckForUpdates))];
        [_updaterController.updater removeObserver:self forKeyPath:NSStringFromSelector(@selector(sessionInProgress))];
    }

    [super dealloc];
}

@end

VersioningController::VersioningController()
    : data::IController{data::ViewState::VersioningView}
    , _manualCheck{}
    , _isFromLoginView{}
{
    sUserCanceled = false;

    @autoreleasepool
    {
        _updaterDelegate = [[AppUpdaterDelegate alloc] init];
        _updaterDelegate.updaterController =
            [[SPUStandardUpdaterController alloc] initWithStartingUpdater:YES
                                                          updaterDelegate:_updaterDelegate
                                                       userDriverDelegate:nil];

        [_updaterDelegate observeCanCheckForUpdatesWithController:this];
    }
}

VersioningController::~VersioningController()
{
    [_updaterDelegate release];
}

[zorgiepoo]

I think your dealloc method is not releasing _updaterController.

Do yourself a favor and compile your file with ARC. This is mentioned in the guide:

// Notes: Compile updater_sparkle.mm with -fobjc-arc because this file assumes Automatic Reference Counting (ARC) is enabled

You won't be able to issue release messages with ARC on and you may have avoided this issue (although you still need to remove KVO observers).

--

When the VersioningController is destroyed which happens during our application workflow after the user requests a version check and the version check is completed..

Also developers typically do not deallocate their updater controller instance (and they create the instance near app launch). Sparkle is designed to "live on" after an update check for two reasons: 1. to let users initiate more update checks in the future (just call checkForUpdates), 2. to let Sparkle check for updates in the background automatically on a periodic basis (if configured to do so; see SUEnableAutomaticChecks).

If your app can create more than one instance of Sparkle's updater for your app during your app's lifetime, I advise against it. sessionInProgress is also not really documented to be observed like you are doing, and you might be misusing it.

During shutdown there's no reason to deallocate either. The OS reclaims the memory when the process exits.