hudeqi commented on code in PR #14729:
URL: https://github.com/apache/kafka/pull/14729#discussion_r1395198255


##########
core/src/main/scala/kafka/network/SocketServer.scala:
##########
@@ -1373,6 +1384,9 @@ private[kafka] class Processor(
     try {
       beginShutdown()
       thread.join()
+      if (!started) {

Review Comment:
   > I don't think I understand what you mean. There's two parallel fixes here 
Acceptor and Processor, they both have the same flaw, and the flaw needs to be 
fixed in both places to prevent leaks.
   > 
   > I'm using synonymous variable and method names names, which is confusing 
because this is all in one file:
   > 
   > * Acceptor close
   > * Acceptor closeAll
   > * Acceptor started
   > * Processor close
   > * Processor closeAll
   > * Processor started
   > 
   > The SocketServer#shutdown calls Acceptor#beginShutdown, Acceptor#close, 
and Acceptor calls Processor#beginShutdown, and Processor#close. Internally, 
the Acceptor and Processor each keep track of a separate started variable, and 
call each closeAll exactly once: In finally if the thread was started, and in 
close if the thread was not.
   
   What I mean is: when the thread of `SocketServer` does not get up, 
`closeAll` will be executed through `started` as false, then there is no need 
to execute `closeAll` once more in `Processor`, even if `Processor` `started` 
in ` is also false? @gharris1727 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to