Github user zwass commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1304#discussion_r126230574
--- Diff: lib/go/thrift/server_socket.go ---
@@ -68,15 +68,18 @@ func (p *TServerSocket) Listen() error {
func (p *TServerSocket) Accept() (TTransport, error) {
p.mu.RLock()
- defer p.mu.RUnlock()
+ listener := p.listener
+ interrupted := p.interrupted
+ p.mu.RUnlock()
--- End diff --
I will admit I also only have limited knowledge of what the code is doing.
I am guessing as to the intended behavior based on the implementation. Is the
expected behavior documented somewhere?
@econner This is something I considered, and here's what I reasoned out:
Ideally, we would ensure that the scenario you describe does not take place
by continuing to hold the lock. In reality, the call to `listener.Accept()` may
block, and if the lock was held this would prevent the `Interrupt()` method
from acquiring the lock (resulting in the "deadlock" experienced previously).
In the case that the instructions are interleaved as you describe, we will
still return an error (albeit a different one) on line 84.
@Jens-G That does look viable to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
