Rather than removing the sandbox locking from sandbox.removeEndpoint() why not have sandbox.addEndpoint() lock like the rest of the sandbox methods that iterate over the endpoint set? Is there a reason for add/remove to be separated out? If not, please put the locking within the method.

Unless there is a reason to not keep the sandbox locking within the methods (see above), then one should acquire the sandbox lock before ranging over the endpoints at the end of this function. Alternately, one could just range over sb.getConnectedEndpoints(). Overall, it probably won't matter based on how this function gets used.

I would have put this comment closer to the actual source line, but given that it is not near a modification github won't let me.

I generally agree, it's better to have the lock inside the functions. It didn't work for me because clearNetworkResources already acquired a lock when calling sb.removeEndpoint and go doesn't seem to like double locking. I wasn't sure if unlocking and relocking around calling sb.removeEndpoint is an option.

Ah, I see. Suggest the following then. Still lock in removeEndpoint(), but have it wrap a method removeEndpointRaw() (rename as desired) that does the real work. Call the latter within the sandbox methods (clearNetworkResources()) and the former outside of sandbox methods (e.g. in sbJoin, sandboxCleanup()). So removeEndpoint() would look something like this:

func (sb *sandbox) removeEndpoint(ep *endpoint) {
        sb.Lock()
        defer sb.Unlock()
        return sb.removeEndpointRaw(ep)
}

and with removeEndpointRaw() would look like removeEndpoint() does in your patch. Makes sense?

Will change the code tomorrow. As a side-effect we then also do locking when calling sb.addEndpoint from sandbox_store.go, which I assume should be fine.
Do you have any objections on #2093 b) and c)?