[PR] [ZEPPELIN-6463] Close the package.json reader in HeliumBundleFactory with try-with-resources [zeppelin]

["big-cir (via GitHub)" <gi...@apache.org>]

big-cir opened a new pull request, #5380:
URL: https://github.com/apache/zeppelin/pull/5380

   ### What is this PR for?
   
   `HeliumBundleFactory.downloadPackage()` stages a Helium package into its bundle directory, either by copying a local directory or by unpacking an npm tarball, and then reads the `package.json` it finds there to pull out the `dependencies` and `main` entries:
   
   ```java
   JsonReader reader = new JsonReader(new FileReader(existingPackageJson));
   Map<String, Object> packageJson = gson.fromJson(reader,
           new TypeToken<Map<String, Object>>(){}.getType());
   ```
   
   The reader is never closed. There is no `close()`, no `finally` and no try-with-resources, and Gson does not close a reader handed to it. Ownership stays with the caller.
   
   The descriptor is therefore released only once the garbage collector reclaims the `FileReader`, because `FileInputStream` registers itself for cleanup. So this is not an unbounded leak, but the release is not deterministic: the descriptor stays open for as long as the `FileReader` goes unreclaimed, which has nothing to do with the point where the parse finishes and the reader stops being useful. The same holds when parsing fails, since a malformed `package.json` makes Gson raise `JsonSyntaxException` and the method exits without closing.
   
   This PR wraps the reader in a try-with-resources so the descriptor is released as soon as parsing finishes. Only `JsonReader` is declared as a resource, since closing it closes the `FileReader` it wraps, which avoids a redundant second close. `packageJson` is declared ahead of the block so the parsed result remains available to the rest of the method. Parsing behaviour and the resulting bundle setup are unchanged, and no signatures or access modifiers change.
   
   ### What type of PR is it?
   Bug Fix
   
   ### Todos
   * [x] Close the `package.json` reader opened in `downloadPackage`
   
   ### What is the Jira issue?
   * https://issues.apache.org/jira/browse/ZEPPELIN-6463
   
   ### How should this be tested?
   
   ```bash
   ./mvnw package -pl zeppelin-server --am \
     -Dtest='HeliumBundleFactoryTest,HeliumTest,HeliumLocalRegistryTest' \
     -DfailIfNoTests=false
   ```
   
   `Tests run: 9, Failures: 0, Errors: 0, Skipped: 0`, and `zeppelin-server` builds.
   
   ### Screenshots (if appropriate)
   N/A
   
   ### Questions:
   * Does the license files need to update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
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: reviews-unsubscribe@zeppelin.apache.org

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

Re: [PR] [ZEPPELIN-6463] Close the package.json reader in HeliumBundleFactory with try-with-resources [zeppelin]

["tbonelee (via GitHub)" <gi...@apache.org>]

tbonelee merged PR #5380:
URL: https://github.com/apache/zeppelin/pull/5380


-- 
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: reviews-unsubscribe@zeppelin.apache.org

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

Re: [PR] [ZEPPELIN-6463] Close the package.json reader in HeliumBundleFactory with try-with-resources [zeppelin]

["tbonelee (via GitHub)" <gi...@apache.org>]

tbonelee commented on PR #5380:
URL: https://github.com/apache/zeppelin/pull/5380#issuecomment-5199592816

   Merged into master


-- 
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: reviews-unsubscribe@zeppelin.apache.org

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