Skip to content

Document the thread safety requirement of SPUUpdater#2746

Closed
sebmarchand wants to merge 1 commit intosparkle-project:2.xfrom
sebmarchand:seb/document_apis
Closed

Document the thread safety requirement of SPUUpdater#2746
sebmarchand wants to merge 1 commit intosparkle-project:2.xfrom
sebmarchand:seb/document_apis

Conversation

@sebmarchand
Copy link
Copy Markdown

This adds some documentation and some runtime checks to the APIs of SPUUpdater to ensure that they only gets used from the main thread as they're not thread safe.

Fixes # 2745

Misc Checklist

  • [X ] My change requires a documentation update on Sparkle's website repository
  • My change requires changes to generate_appcast, generate_keys, or sign_update

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • [ X] My own app
  • Other (please specify)

I ensured that the code was not crashing because of the new checks.

macOS version tested: 15.5

- (void)checkForUpdatesInBackground
{
if (![NSThread isMainThread]) {
SULog(SULogLevelError, @"Error: -checkForUpdatesInBackground can only be called on the main thread");
Copy link
Copy Markdown
Member

@zorgiepoo zorgiepoo Jul 29, 2025

Choose a reason for hiding this comment

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

I suggest using full method style like -[SPUUpdater checkForUpdatesInBackground]

Same for other methods.

I'm also thinking we could just dispatch_async(dispatch_get_main_queue(), ^{ [self checkForUpdatesInBackground] }) anyway to be nice for these three methods.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sounds good! We're in a crunch at work so I'm a little bit busy, but I'll do this soon once I have some spare cycles!

@zorgiepoo
Copy link
Copy Markdown
Member

zorgiepoo commented Aug 17, 2025

I incorporated these changes and added some more checks in #2754. Thanks!

@zorgiepoo zorgiepoo closed this Aug 17, 2025
@sebmarchand
Copy link
Copy Markdown
Author

Thanks! I'm sorry it took so long to address your comments! We're in a big crunch at work! Really appreciate your help here and your help on the ticket I opened originally!

@zorgiepoo
Copy link
Copy Markdown
Member

No worries, had some of my own touches, and haven't been addressing things too promptly either.

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.

2 participants