migueldeicaza / SwiftTerm

Xterm/VT100 Terminal emulator in Swift
MIT License
978 stars 145 forks source link

Use weak ref to fix crash when process exits after view is released #293

Closed kdrag0n closed 1 year ago

kdrag0n commented 1 year ago

If a LocalProcess is terminated or exits right before the hosting LocalProcessTerminalView is released, the processTerminated call crashes the app with the following error:

Fatal error: Attempted to read an unowned reference but the object was already deallocated

All usages of delegate already have nil checks, so just change the variable from unowned to weak to fix the issue. There's no point in calling the delegate when it no longer exists.

migueldeicaza commented 1 year ago

Thank you for your patch!

migueldeicaza commented 1 year ago

Mhm, I wonder why changing unowned to weak fixes this?

kdrag0n commented 1 year ago

I think it's because unowned is strict and crashes if you try to access a reference to a freed object. weak returns nil instead, which is more in line with the expected behavior for delegates.

Lakr233 commented 1 year ago

unowned should be banned for 99% of cases, it is dangerous as we do not have, most of time, control to ARC overtime so the pointer should be set to nil when release (weak) instead of keep it (unowned)