C0urante commented on code in PR #12321:
URL: https://github.com/apache/kafka/pull/12321#discussion_r909055766
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -997,7 +997,9 @@ public interface UncheckedCloseable extends AutoCloseable {
}
/**
- * Closes {@code closeable} and if an exception is thrown, it is logged at
the WARN level.
+ * Closes {@code closeable} and if an exception is thrown, it is logged at
the WARN level. Note that, if
+ * a method reference of null object is passed as closeable to this
method, then that leads to NPE and
+ * the close() doesn't happen as expected.
Review Comment:
I think it might help to illustrate with an example, possibly with something
like `sourceTask::stop`.
It also seems like the language here suggests that this is the fault of
`closeQuietly`, which isn't the case; in reality, an NPE will be thrown at the
calling side before this method can even be invoked. It's also important to
note that, instead of `close` not being invoked (which is a non-issue if the
object is null anyways), the issue is that the caller will experience an NPE if
they're not careful about how they use this method.
Maybe something like this would help clarify that?
```
* <b>Be cautious when passing method references as an argument.</b> For
example:
* <p>
* {@code closeQuietly(task::stop, "source task");}
* <p>
* Although this method gracefully handles null {@link AutoCloseable}
objects, attempts to take a method
* reference from a null object will result in a {@link
NullPointerException}. In the example code above,
* it would be the caller's responsibility to ensure that {@code task}
was non-null before attempting to
* use a method reference from it.
```
--
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
