Skip to content

Commit

Permalink
Update LineNumbers to ignore all kinds of exceptions when reading cla…
Browse files Browse the repository at this point in the history
…sses with ASM, so Guice is less finicky about the precise .class version & ASM version. A while ago I had tried to do this by ignoring UnsupportedOperationException, but per #1654, ASM doesn't always throw UOE.

This change will log a single failure when Guice encounters an exception while reading classfiles and warn that ASM may be out of date.

The consequences of this failing _all_ line number reading (due to, say, an accidental bug introduced while parsing classfiles) is that Guice won't emit line numbers for bind statements in modules (as binding source locations). A number of other tests would fail in that scenario, warning us that something is off.

PiperOrigin-RevId: 524365509
  • Loading branch information
sameb authored and Guice Team committed Apr 14, 2023
1 parent d98a4de commit 7f8cd8c
Showing 1 changed file with 17 additions and 2 deletions.
19 changes: 17 additions & 2 deletions core/src/com/google/inject/internal/util/LineNumbers.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
Expand All @@ -42,6 +44,9 @@
*/
final class LineNumbers {

private static final Logger logger = Logger.getLogger(LineNumbers.class.getName());
private static volatile boolean alreadyLoggedReadingFailure;

private static final int ASM_API_LEVEL = Opcodes.ASM9;

private final Class<?> type;
Expand All @@ -67,10 +72,20 @@ public LineNumbers(Class<?> type) throws IOException {
if (in != null) {
try {
new ClassReader(in).accept(new LineNumberReader(), ClassReader.SKIP_FRAMES);
} catch (UnsupportedOperationException ignored) {
} catch (Exception ignored) {
// We may be trying to inspect classes that were compiled with a more recent version
// of javac than our ASM supports. If that happens, just ignore the class and don't
// capture line numbers.
// capture line numbers. But log the failure so folks know something's off.
// (Only log it once, though, to avoid spam. It's OK if concurrent access makes this
// happen more than once.)
if (!alreadyLoggedReadingFailure) {
alreadyLoggedReadingFailure = true;
logger.log(
Level.WARNING,
"Failed loading line numbers. ASM is probably out of date. Further failures won't"
+ " be logged.",
ignored);
}
} finally {
try {
in.close();
Expand Down

0 comments on commit 7f8cd8c

Please sign in to comment.