Ranchero-Software / NetNewsWire

RSS reader for macOS and iOS.
https://netnewswire.com/
MIT License
8.22k stars 524 forks source link

Reproducible read-after-free crash when adding iCloud account #3606

Open winniequinn opened 2 years ago

winniequinn commented 2 years ago

After installing NetNewsWire on iOS and adding an iCloud account there, I installed 6.1 on macOS and attempted to do the same. Each time I attempt to do so, I immediately crash just as the "Use your iCloud account." dialog begins to show up.

Deleting all NetNewsWire data in "~/Library/Containers" and deleting all NetNewsWire data in iCloud does not resolve the problem. This is my first time ever attempting to use the application, so lingering data from a previous version cannot be an issue.

See the crash report for more info.


Update: It appears that I receive the same crash when attempting to add any type of account.

vincode-io commented 2 years ago

Thanks for the crash report. It looks like it might be some kind of SwiftUI bug.

Have you tried rebooting and then adding the iCloud account then? We've seen in recent versions of macOS that rebooting will sometimes clear up weird macOS bugs.

winniequinn commented 2 years ago

@vincode-io: Restarting did not resolve the issue, I'm afraid.

vincode-io commented 2 years ago

I'm just grasping at straws at this point. Let's try a couple things.

Delete everything in ~/Library/Containers/com.rancharo.NetNewsWire-Evergreen. That is where NetNewsWire 6 keeps its data.

If that doesn't work, you might try upgrading to macOS 12.4. This might be a SwiftUI bug in 12.3. I only have access right now to a M1 Mac running 12.4 and I can't recreate this problem there.

Another thought just occurred to me. You do have iCloud Drive enabled, right? Not having it enabled shouldn't cause a crash, but it might cause NetNewsWire to throw up a dialog that is causing the problem. I would test it myself, but I'm not able to disable iCloud Drive right now (I'm only on cellular, so disabling it would take me forever to get back to normal).

Thanks for your help in tracking this down.

winniequinn commented 2 years ago
winniequinn commented 2 years ago

Thanks for trying to get to the bottom of this! I'll post again after I update to 12.4.

vincode-io commented 2 years ago

I'm keeping my fingers crossed that the upgrade fixes it for you.

You shouldn't need the iCloud Drive folders stuff, so that isn't likely the problem.

winniequinn commented 2 years ago

I'm getting the same crash with 12.4, unfortunately.

vincode-io commented 2 years ago

The crashlogs looks the same then?

vincode-io commented 2 years ago

Just to be clear, it is when you hit "Continue" in the below screenshot that you briefly see the dialog and then it crashes, right?

Screen Shot 2022-06-20 at 2 38 23 PM
winniequinn commented 2 years ago

That is correct. It crashes just as the dialog starts to fade in; I managed a screenshot after a few attempts:

Screen Shot 2022-06-20 at 5 15 14 PM

To be clear, the crash occurs almost immediately. There's no time at all to click either button.

winniequinn commented 2 years ago

I can imagine this might be a tough one to debug given the stack trace. If you don't have anything to go on, I'll try to build the app and debug it with zombies enabled to see if I can get you more info.

vincode-io commented 2 years ago

Wow. I'm at a loss as to what could be happening. It looks specific to your environment, but what is different?

Do you want to try running this in Xcode to see if you get stopped in the debugger someplace helpful? It is a real pain to get NetNewsWire set up to use full iCloud capabilities, but I can give you a simple change that will at least get the dialog to show.

vincode-io commented 2 years ago

We crossed comments. Give a second and I'll let you know what to change in NetNewsWire so that you can test this with the standard build instructions.

vincode-io commented 2 years ago

Just remove the .cloudKit entry from this line to allow you to click the iCloud option in the -dev build.

https://github.com/Ranchero-Software/NetNewsWire/blob/876b03340012f3aa231ee2f33bf29ec4ff834df3/Account/Sources/Account/Account.swift#L49

Good luck and let me know if you need. any help. If I'm not responding, you can ask for help in the #work channel of our Slack.

winniequinn commented 2 years ago

It seems that I get the same crash when I try to add any type of account, it turns out. I also get it simply by clicking "Cancel" on the "Choose an account type to add…" screen.

I was able to fix it by holding the NSHostingController created in the addAccount method of AccountsPreferencesViewController in a new instance variable—one which would seem to serve a role similar to that of the existing addAccountWindowController instance variable. Whether or not this is the correct fix, I have not had time to investigate, but it does work around the problem, at least.

vincode-io commented 2 years ago

Thanks for figuring this out! That definitely looks like a problem to me. I'm honestly baffled as to why we don't see this all the time. Your solution also sounds correct.

Do you want to submit a pull request for this? We would really appreciate it.

winniequinn commented 2 years ago

Glad I was able to get to the bottom of it!

I wish I could submit a PR today, but I can't do so due to my relationship with my present employer: I'd need to get it cleared first. I'm currently away from work until July, so, as embarrassed as I am by this situation, you may just want to make the fix yourself unless you're okay with waiting a few weeks. (If you are though, I'll certainly get you a PR as soon as I can!)

winniequinn commented 2 years ago

I believe you have the same problem in the enableExtensionPoints and enableExtensionPointFromSelection methods of ExtensionPointPreferencesViewController (although I can't trigger a crash, for whatever reason).

vincode-io commented 2 years ago

I don't know yet when our next bug release for the Mac version will be. If you aren't able get to it, I'll make the change and just hope for the best since I can't recreate it.

I'll also take a look at the other functions you mentioned.

winniequinn commented 2 years ago

@vincode-io: #3670 should take care of this! While I cannot confirm that it fixes the crash I had when adding an iCloud account specifically (as I did not attempt to build a version of NWW with iCloud support), it does fix the crash I had when attempting to add any other type of account; I imagine it covers the iCloud case too.

winniequinn commented 2 years ago

Note: While I did try changing isDeveloperRestricted as suggested, that alone did not result in iCloud showing up as an account option in a development build. No matter though: I am still reasonably confident that the issue is fixed as I had the same issue with all other account types.

brentsimmons commented 1 month ago

Reopening because this needs additional verification.