Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5629

Unclosed RandomAccessFile in StaticFileServerHandler#respondAsLeader()

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: Webfrontend
    • Labels:
      None

      Description

          final RandomAccessFile raf;
          try {
            raf = new RandomAccessFile(file, "r");
      ...
          long fileLength = raf.length();
      

      The RandomAccessFile should be closed upon return from method.

        Issue Links

          Activity

          Hide
          mylog00 Dmitrii Kniazev added a comment -

          This code has comment

          StaticFileServerHandler.java
          // Don't need to close this manually. Netty's DefaultFileRegion will take care of it.
          final RandomAccessFile raf;
          try {
          	raf = new RandomAccessFile(file, "r");
          }
          catch (FileNotFoundException e) {
          	sendError(ctx, NOT_FOUND);
          	return;
          }
          long fileLength = raf.length();
          

          I think this bug may be closed

          Show
          mylog00 Dmitrii Kniazev added a comment - This code has comment StaticFileServerHandler.java // Don't need to close this manually. Netty's DefaultFileRegion will take care of it. final RandomAccessFile raf; try { raf = new RandomAccessFile(file, "r" ); } catch (FileNotFoundException e) { sendError(ctx, NOT_FOUND); return ; } long fileLength = raf.length(); I think this bug may be closed
          Hide
          yuzhihong@gmail.com Ted Yu added a comment - - edited

          RandomAccessFile#length() may throw IOE.
          raf is used in the following code path where DefaultFileRegion is not involved:

              } else {
                lastContentFuture = ctx.writeAndFlush(new HttpChunkedInput(new ChunkedFile(raf, 0, fileLength, 8192)),
          

          It is good practice to close RandomAccessFile in all code paths.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - - edited RandomAccessFile#length() may throw IOE. raf is used in the following code path where DefaultFileRegion is not involved: } else { lastContentFuture = ctx.writeAndFlush( new HttpChunkedInput( new ChunkedFile(raf, 0, fileLength, 8192)), It is good practice to close RandomAccessFile in all code paths.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

          https://github.com/apache/flink/pull/3678

          FLINK-5629 [runtime-web] Close RAF in FileServerHandlers when exception occurs

          This PR makes sure that the RandomAccessFile opened by FileServerhandlers are closed if an exception occurs after opening it. If no exception occurs `respondWithFile` we still rely on netty to properly close the file.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/zentol/flink 5629_raf_close

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3678.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3678



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/3678 FLINK-5629 [runtime-web] Close RAF in FileServerHandlers when exception occurs This PR makes sure that the RandomAccessFile opened by FileServerhandlers are closed if an exception occurs after opening it. If no exception occurs `respondWithFile` we still rely on netty to properly close the file. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 5629_raf_close Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3678.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3678
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/3678

          This PR also fixed FLINK-6172.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3678 This PR also fixed FLINK-6172 .
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3678

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3678
          Hide
          Zentol Chesnay Schepler added a comment -

          1.3: be26f7ed1e2b97bc2e6ab06d6267f8d542d78aee

          Show
          Zentol Chesnay Schepler added a comment - 1.3: be26f7ed1e2b97bc2e6ab06d6267f8d542d78aee

            People

            • Assignee:
              Zentol Chesnay Schepler
              Reporter:
              yuzhihong@gmail.com Ted Yu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development