Fix crash caused by cleanup() being called from multiple threads #476
Conversation
If InputStream/OutputStream .close() or *SetDispatchQueue is called from two threads at the same time, it will cause a crash. This adds a lock to prevent the cleanup() method from executing concurrently. Once the first one is complete, inputStream and outputStream and nil and so simultaneous attempts to cleanup() become noops.
|
@fassko or @flagoworld, have either of you tried this with any success, or is it a hunch that this could / should fix it? We've got quite a lot of reports of a similar looking crash |
| @@ -248,6 +250,8 @@ open class FoundationStream : NSObject, WSStream, StreamDelegate { | |||
| } | |||
|
|
|||
| public func cleanup() { | |||
| cleanupMutex.lock() | |||
There was a problem hiding this comment.
The lock will need to be applied universally to protect access to the vars protected here or it won't help much. I would also use an NSRecursiveLock unless you can fully understand / constrain the calling model that exists.
There was a problem hiding this comment.
I don't think recursive lock is necessary here, but I understand what you are saying about protecting all the variables that are used within the lock across the class. That said, so far in practice this has eliminated the crashes without issue. Provide a better implementation for Dalton so we can get a fix merged!
|
@hamchapman |
|
I spent some time to cleanup the multiple NSLock objects and also help with this issue. I release new version ( |
|
It's been a while and based on the thumbs up feedback, I'm gonna assume we fix this. Thanks again! |
If InputStream/OutputStream .close() or *SetDispatchQueue is called
from two threads at the same time, it will cause a crash. This adds a
lock to prevent the cleanup() method from executing concurrently. Once
the first one is complete, inputStream and outputStream and nil and so
simultaneous attempts to cleanup() become noops.
Is it possible that locking inside cleanup() has some unintended side-effects? Take a look at let me know. I am not as familiar with the codebase...