[
https://issues.apache.org/jira/browse/GROOVY-12385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113282#comment-18113282
]
ASF GitHub Bot commented on GROOVY-12385:
-----------------------------------------
Copilot commented on code in PR #2907:
URL: https://github.com/apache/groovy/pull/2907#discussion_r3967741559
##########
src/test/groovy/org/apache/groovy/runtime/async/ScopedLocalTest.groovy:
##########
@@ -836,4 +836,13 @@ class ScopedLocalTest {
assertEquals('computed', result)
}
}
+
+ @Test
+ @DisplayName('GROOVY-12385: ScopedValue detection reads the feature
version defensively')
+ void featureVersionMatchesTheRuntimeOnAJvm() {
+ def bindings = ScopedLocal.declaredClasses.find { it.simpleName ==
'ScopedValueBindings' }
+ def featureVersion = bindings.getDeclaredMethod('featureVersion')
+ featureVersion.accessible = true
+ assertEquals(Runtime.version().feature(), featureVersion.invoke(null))
Review Comment:
This test directly calls `Runtime.version()`, which will throw on runtimes
that lack it (the same scenario the production code is now handling). To keep
the test aligned with the fix and avoid hard failures in such environments,
guard the assertion with a try/catch (or an assumption) and assert the fallback
behavior (e.g., `0`) when `Runtime.version()` is unavailable.
##########
src/main/java/org/apache/groovy/runtime/async/ScopedLocal.java:
##########
@@ -658,14 +658,28 @@ private ScopedValueBindings(boolean available,
MethodHandle newInstance, MethodH
this.carrierRun = carrierRun;
}
+ /**
+ * The Java feature version, or 0 on a runtime without {@code
Runtime.version()}
+ * (Android's ART), which has no {@code ScopedValue} either
(GROOVY-12385).
+ * Kept local rather than shared with the VM plugin factory: this
class is
+ * repackaged into groovy-concurrent-java, which must not depend on it.
+ */
+ static int featureVersion() {
+ try {
+ return Runtime.version().feature();
+ } catch (Throwable t) {
Review Comment:
Catching `Throwable` is overly broad and can inadvertently mask serious
problems (e.g., `OutOfMemoryError`, `StackOverflowError`) unrelated to feature
detection. Narrow the catch to the specific likely failures for
missing/unsupported `Runtime.version()` (e.g., `NoSuchMethodError`,
`LinkageError`, and possibly `SecurityException`) so fatal VM conditions still
surface.
> ScopedLocal: ScopedValue detection must not assume Runtime.version()
> --------------------------------------------------------------------
>
> Key: GROOVY-12385
> URL: https://issues.apache.org/jira/browse/GROOVY-12385
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Involves: a local try/catch helper returning zero when the method is absent.
> It has to stay local rather than reuse item 1, because groovy-concurrent-java
> repackages this class without VMPluginFactory and has a self-containment test.
> Impact on normal usage: none.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
