[ 
https://issues.apache.org/jira/browse/GROOVY-12404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18114720#comment-18114720
 ] 

ASF GitHub Bot commented on GROOVY-12404:
-----------------------------------------

codecov-commenter commented on PR #2928:
URL: https://github.com/apache/groovy/pull/2928#issuecomment-5651088873

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2928?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :x: Patch coverage is `94.44444%` with `1 line` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 71.1941%. Comparing base 
([`3246257`](https://app.codecov.io/gh/apache/groovy/commit/32462579e2689374115525c2ac3e50a1ad86dfb5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`d38a1c2`](https://app.codecov.io/gh/apache/groovy/commit/d38a1c289cce37d1697972d2ecad446abb40a686?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 8 commits behind head on master.
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2928?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[src/main/java/groovy/util/GroovyScriptEngine.java](https://app.codecov.io/gh/apache/groovy/pull/2928?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Futil%2FGroovyScriptEngine.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvdXRpbC9Hcm9vdnlTY3JpcHRFbmdpbmUuamF2YQ==)
 | 94.4444% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2928/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2928        +/-   ##
   ==================================================
   - Coverage     71.1971%   71.1941%   -0.0030%     
   - Complexity      37729      37736         +7     
   ==================================================
     Files            1584       1584                
     Lines          136500     136517        +17     
     Branches        25361      25368         +7     
   ==================================================
   + Hits            97184      97192         +8     
   - Misses          30546      30547         +1     
   - Partials         8770       8778         +8     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2928?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/main/java/groovy/util/GroovyScriptEngine.java](https://app.codecov.io/gh/apache/groovy/pull/2928?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Futil%2FGroovyScriptEngine.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvdXRpbC9Hcm9vdnlTY3JpcHRFbmdpbmUuamF2YQ==)
 | `87.7193% <94.4444%> (+2.3622%)` | :arrow_up: |
   
   ... and [11 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2928/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   - :package: [JS Bundle 
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save 
yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>




> GroovyScriptEngine resource loader leaks URLConnections and ignores a custom 
> ResourceConnector
> ----------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12404
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12404
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Daniel Sun
>            Priority: Major
>
> {{groovy.util.GroovyScriptEngine}} resolves dependent sources through 
> {{ResourceConnector.getResourceConnection}}, but several call sites keep only 
> the URL and never close the {{URLConnection}}.
> {{URLConnection}} has no {{close()}} method. The engine already has 
> {{forceClose}} for this (see GROOVY-7186) and uses it from 
> {{loadScriptByName}}. The resource-loader path does not. 
> {{ClassNodeResolver.findClassNode}} only closes the connection when the 
> source is newer; the cache-hit path leaks. Long-running hosts that resolve 
> many scripts can exhaust file descriptors.
> The same resource loader has two further gaps:
> # When the parent class loader is a {{GroovyClassLoader}}, 
> {{ScriptClassLoader}} never calls {{setResLoader()}}. {{new 
> GroovyScriptEngine(rc, new GroovyClassLoader())}} then ignores {{rc}} for 
> {{loadClass}} / {{Class.forName}}.
> # Packaged names are mapped with {{File.separatorChar}}. On Windows the 
> resource name uses a backslash as the separator instead of 
> {{com/example/Foo.groovy}}, which is what {{ClassNodeResolver}} and 
> {{GroovyClassLoader}} already use.
> h3. How to reproduce
> h4. Custom ResourceConnector ignored
> {code:java}
> File dir = File.createTempDir()
> new File(dir, 'Helper.groovy').text = 'class Helper { def ping() { "pong" } }'
> ResourceConnector rc = { String name ->
>     File f = new File(dir, name)
>     if (!f.exists()) throw new ResourceException('missing ' + name)
>     f.toURI().toURL().openConnection()
> }
> def engine = new GroovyScriptEngine(rc, new GroovyClassLoader())
> engine.groovyClassLoader.loadClass('Helper')
> {code}
> h4. Resource-loader connection not closed
> With {{Foo.groovy}} present, install a {{ResourceConnector}} whose 
> {{URLConnection.getInputStream()}} records that it was called, then:
> {code:java}
> engine.groovyClassLoader.resourceLoader.loadGroovySource('Foo')
> {code}
> {{getInputStream()}} is never invoked, so {{forceClose}} did not run.
> h3. Expected result
> * Connections obtained only for their URL are closed with {{forceClose}}.
> * A custom {{ResourceConnector}} is used even when the parent is a 
> {{GroovyClassLoader}}.
> * Resource names for packaged classes use a forward slash on every platform.
> h3. Actual result
> * The resource-loader {{URLConnection}} is left open.
> * {{loadClass('Helper')}} throws {{ClassNotFoundException}} even though 
> {{Helper.groovy}} is visible to the connector.
> * On Windows, connectors that do not rewrite backslashes to slashes fail to 
> find packaged scripts.
> h3. See also
> GROOVY-7186 addressed a similar leak in {{getResourceConnection}} and added 
> {{forceClose}}. The resource-loader path and the cache-hit branch of 
> {{findClassNode}} were not covered.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to