DanielLeens commented on PR #10678: URL: https://github.com/apache/seatunnel/pull/10678#issuecomment-5207549735
@Thanks @nzw921rx for the follow-up — I independently reproduced this before replying. I re-ran a standalone `URLConnection.setDefaultUseCaches` probe on JDK 8u171 (same major version as your Corretto 8.472 run) against a fresh HTTP connection opened before and after calling `setDefaultUseCaches(false)` on a `jar:` connection, exactly what `disableJarUrlCache()` does at `DefaultClassLoaderService.java:67-73` (specifically the `connection.setDefaultUseCaches(false)` call on line 71): ``` http-before=true http-after=false file-after=false jar-after=false ``` This matches your numbers exactly. On JDK 8, `URLConnection.setDefaultUseCaches(boolean)` is not protocol-scoped — it mutates a single static field shared by every subsequently opened `URLConnection`, regardless of protocol. Calling it on a `jar:` connection at service construction time flips the JVM-wide default for `http:`/`file:`/anything else too. This is a real correction to my own earlier review. I flagged the same call site as "probably fine in practice ... standard Tomcat-style leak-prevention idiom" and treated it as a non-blocking note. That framing was wrong: the JDK 8 `URLConnection` API itself has no protocol scoping, and here it runs unconditionally inside `SeaTunnelServer.init()` before any connector code has a chance to set its own caching policy. Your call-site evidence (`RestService.getConnectionPost`/`getConnectionGet`, `BaseLogService.sendGet`) is a good concrete illustration — none of them call `setUseCaches` explicitly, so they silently inherit `useCaches=false` from this one constructor call. I'm revising my earlier assessment from a non-blocking style note to agreeing this is a real, independent blocker. +1. Net effect: two source-side blockers remain open on the current head (`89ff5c3f63c2`), both requiring an actual code change rather than a doc-only fix: 1. JDK 9+ `--add-opens java.base/jdk.internal.loader=ALL-UNNAMED` documentation/warning gap (raised in my prior review, still unaddressed). 2. JDK 8 JVM-wide `useCaches` default mutation in `disableJarUrlCache()` (raised here by @nzw921rx, confirmed above). @knight6236 no need to guess which to tackle first — both are independent and can land in the same commit. Happy to re-review as soon as a new commit is pushed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
