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.
--
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]
