On Tue, 8 Apr 2025 08:18:36 GMT, Danish Nawab <d...@openjdk.org> wrote:
> ## Description > > https://bugs.openjdk.org/browse/JDK-8353840 > > ### Existing behavior > Log the error message and terminate in case of missing system class > > > $ jnativescan --class-path reactor-core-3.7.4.jar; echo "Exit code: $?" > > ERROR: Error while processing method: > reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::get()String > CAUSED BY: System class can not be found: sun.misc.JavaLangAccess > Exit code: 1 > > > ### New behavior > Still log the error message about the missing system class, but continue the > analysis > > > $ build/macosx-aarch64-server-release/jdk/bin/jnativescan --class-path > reactor-core-3.7.4.jar; echo "Exit code: $?" > > <no restricted methods> > Error while processing method: > reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::get()String: > System class can not be found: sun.misc.JavaLangAccess > Error while processing method: > reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::<clinit>()void: > System class can not be found: sun.misc.SharedSecrets > Error while processing method: > reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory::<clinit>()void: > System class can not be found: sun.misc.SharedSecrets > Exit code: 0 > > > ## Design choices > > Propagate `err` all the way to `NativeMethodFinder` which can log the error > to it, but continue with the analysis > > ### Alternatives considered > > - Instead of propagating `err` downstream, adapt the outer try-catch in > `com.sun.tools.jnativescan.Main#run`. > - Con: This would require complicated error recovery synchronization > between Main and `JNativeScanTask` to resume the scanning after the exception > - Instead of adapting the catch block in > `com.sun.tools.jnativescan.NativeMethodFinder#findAll`, don't even surface > the exception from > `com.sun.tools.jnativescan.ClassResolver.SystemModuleClassResolver#lookup` > rather log it right in `ClassResolver` > - Con: We don't have access to `MethodRef`/`MethodModel` in > ClassResolver#lookup which will make the error message less detailed/useful > > ### stdout vs stderr > > One could argue that since this is a non-terminal error, perhaps it should be > logged to stdout. However, my thinking was that while it may be non-terminal, > it is still an "error", and thus should be logged to stderr. I am happy to > revisit this decision, if needed. > > ## Testing > > The existing test `TestMissingSystemClass#testSingleJarClassPath` has ... src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line 79: > 77: try(ClassResolver classesToScan = > ClassResolver.forClassFileSources(toScan, version); > 78: ClassResolver systemClassResolver = > ClassResolver.forSystemModules(version)) { > 79: NativeMethodFinder finder = NativeMethodFinder.create(err, > classesToScan, systemClassResolver); e.g. Suggestion: Set<String> errors = new HashSet<>(); // print these at the end Diagnostics diagnostics = (context, error) -> errors.add("Error while processing " + context + ": " + error.getMessage(); NativeMethodFinder finder = NativeMethodFinder.create(diagnostics, classesToScan, systemClassResolver); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24499#discussion_r2033057140
