agingade commented on code in PR #7582:
URL: https://github.com/apache/geode/pull/7582#discussion_r849949672


##########
geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java:
##########
@@ -117,13 +117,21 @@ private String status(final File workingDir, final String 
statusRequestFileName,
     File statusFile = new File(workingDir, statusFileName);
     AtomicReference<String> statusRef = new AtomicReference<>();
 
-    ControlRequestHandler statusHandler = () -> {
-      // read the statusFile
-      StringBuilder lines = new StringBuilder();
-      try (BufferedReader reader = new BufferedReader(new 
FileReader(statusFile))) {
-        reader.lines().forEach(lines::append);
-      } finally {
-        statusRef.set(lines.toString());
+    ControlRequestHandler statusHandler = new ControlRequestHandler() {
+      @Override
+      public void handleRequest() throws IOException {
+        handleRequest(null);
+      }
+
+      @Override
+      public void handleRequest(FileReader fileReader) throws IOException {
+        // read the statusFile
+        StringBuilder lines = new StringBuilder();
+        try (BufferedReader reader = new BufferedReader(fileReader)) {

Review Comment:
   According BufferReader(FileReader) implementation; it will thrown NPE when 
null arg is passed. 



##########
geode-core/src/main/java/org/apache/geode/internal/process/ControlFileWatchdog.java:
##########
@@ -175,6 +185,21 @@ private String createThreadName() {
    * Defines the callback to be invoked when the control file exists.
    */
   interface ControlRequestHandler {
+    /**
+     * This is the form that is called from {@link 
ControlFileWatchdog#handleRequest(FileReader)}.
+     * Not all implementations require the {@link FileReader}, so they may 
continue implementing the
+     * no-args form which is called from this method by default.
+     *
+     * @param fileReader a {@link FileReader} to read the status file.
+     */
+    default void handleRequest(FileReader fileReader) throws IOException {

Review Comment:
   I am unable to see the need for this. 
   The ControlRequestHandler implementing could have implemented all the needed 
work in handleRequest() it self; rather than in handleRequest(FileReader).
   I may be missing something. I will let experts to weigh on this.



-- 
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: notifications-unsubscr...@geode.apache.org

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

Reply via email to