Fix socket.infoHashes access error #208
Conversation
| peer.socket = null | ||
| } catch (err) {} | ||
| } | ||
| peer.socket = null |
There was a problem hiding this comment.
This entire section isn't actually necessary, I realized. It is okay for a client to have a websocket open to the server but to not be in any swarms. Because they might join another swarm in the future.
When the client first connects, they are not in any swarms and we are okay with that. So, there's no reason to close it preemptively here. We always do cleanup once the client closes the socket.
There was a problem hiding this comment.
Why not use delete? I think we've discussed it before but I couldn't recall the reason.
There was a problem hiding this comment.
In this situation, either way works fine. The whole peer object is going to be garbage-collected once all the references to it are gone.
The situation you're referring to is when trying to remove a property from an instance of a class (i.e. something with a prototype). In that case, it's better to use object.prop = null since delete object.prop will cause a de-optimization because it changes the "shape" of the object in memory.
Using delete is fine on object literals {} though.
There was a problem hiding this comment.
Ah, alright, thanks!
So why even nullify?, it's been deleted from the lru, once this block runs it should be gc'd
There was a problem hiding this comment.
You make a good point. It's not necessary, but I think it doesn't hurt to be explicit where we can. We could also set any references to peer to null but we don't keep any of those, other than what's in the LRU cache.
|
Released as 8.5.1 |
Fixes: #205