Frugghi / SwiftSH

A Swift SSH framework that wraps libssh2.
MIT License
194 stars 71 forks source link

unowned vs weak #2

Closed jpalten closed 6 years ago

jpalten commented 6 years ago

I see a lot of unowned where weak should be used. Would you like me to look into that and create a merge request for that?

Frugghi commented 6 years ago

If you want to open a PR I'll review it.

If you're referring to the unowned in the DispatchSource handlers, I don't see how they could be dangerous: if the object that holds the reference to the DispatchSource is deallocated, the DispatchSource is deallocated too.

atticus commented 6 years ago

First test of the code, and I'm seeing issues with the unowned references. In the console:

"Fatal error: Attempted to read an unowned reference but object 0x7b480005d780 was already deallocated"

Died in the SSH Queue at

socketSource.setCancelHandler { [unowned self] in self.close() }

atticus commented 6 years ago

To replicate, the command I was using fell out of scope (wasn't strong reference) before the execute completed. The issue is that instead of the command cleaning up, it took down the app due to a bad unowned pointer. Changing references to weak fixed the issue for me.

jpalten commented 6 years ago

I created the PR, hope it helps!