Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/2019#discussion_r16727939
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -280,42 +280,46 @@ private[spark] class ConnectionManager(
}
while(!keyInterestChangeRequests.isEmpty) {
+ // Expect key interested in OP_ACCEPT is not change its interest
val (key, ops) = keyInterestChangeRequests.dequeue()
-
try {
- if (key.isValid) {
+ key.synchronized {
--- End diff --
I'm a little concerned that this isn't really adding much benefit.
Connection.close() doesn't synchronize on anything, so it can still be
called while this lock is held, as far as I understand the code. So you could
still execute this code all the way to L291 and then get a
CancelledKeyException the same way. So you're maybe narrowing the conditions in
which you'd get the exception, but it's still possible to get it.
I guess this will be similar to Mridul's comment, but: if the issue is that
we're logging these exceptions, how about just demoting them? If we're
comfortable that the code is doing the right thing and handling the errors
appropriately, then the log messages are not that interesting.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
