JDK behavior change breaks Listeners that override a generic method

XMLWordPrintable

    • Type: Bug
    • Resolution: Fixed
    • Priority: Major
    • None
    • Affects Version/s: None
    • Environment:

      Java 7u80 (or above)
      Java 8 (any version)

      Consider the following example:

      Base.java
      public abstract class Base<T extends Event> implements Listener {
    abstract void method(T event);
}
      Derived.java
      public class Derived extends Base<BlockPlaceEvent> {
    @EventHandler
    void onEvent(BlockPlaceEvent event) {}
}

      If you examine Derived with javap, you discover that it actually has two methods.

      Compiled from "Derived.java"
public class Derived extends Base<org.bukkit.event.block.BlockPlaceEvent> {
  public Derived();
  void onEvent(org.bukkit.event.block.BlockPlaceEvent);
  void onEvent(org.bukkit.event.Event);
}

      In Java 7u79 and earlier, the @EventHandler annotation only exists on the void onEvent(org.bukkit.event.block.BlockPlaceEvent) method. However, a change was made to the JDK (see https://bugs.openjdk.java.net/browse/JDK-6695379) to copy the annotation to the overridden method, that being void onEvent(org.bukkit.event.Event).

      Bukkit reflectively looks for the getHandlerList method for any class in the parameter list of a function annotated with @EventHandler. In earlier versions of Java 7, it would only notice the void onEvent(BlockPlaceEvent) version and successfully register the class. In code compiled with the above mentioned versions, @EventHandler is copied to the void onEvent(Event) method, and then an exception is thrown when Bukkit can't find the getHandlerList method.

          [SPIGOT-893] JDK behavior change breaks Listeners that override a generic method

          Jonas Konrad added a comment -

          Yes, you're right, I forgot that the bukkit event system doesn't invoke super handlers with the event.

          However, the test still works - it fails on the old version with the exception mentioned in the report, and passes with the fix applied. I suppose you can see it as a test that only bridge and synthetic methods get filtered

          Jonas Konrad added a comment - Yes, you're right, I forgot that the bukkit event system doesn't invoke super handlers with the event. However, the test still works - it fails on the old version with the exception mentioned in the report, and passes with the fix applied. I suppose you can see it as a test that only bridge and synthetic methods get filtered

          Andrew Shulman added a comment -

          Thanks for the quick fix. It's worth noting, though, that your test won't do exactly what you think it will. If you decompile the bridge method

          javap -v Derived
            void method(org.bukkit.event.Event);
    descriptor: (Lorg/bukkit/event/Event;)V
    flags: ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: checkcast     #2                  // class org/bukkit/event/block/BlockPlaceEvent
         5: invokevirtual #3                  // Method method:(Lorg/bukkit/event/block/BlockPlaceEvent;)V
         8: return
      LineNumberTable:
        line 2: 0
    RuntimeVisibleAnnotations:
      0: #13()

          All it does is call the overriding method, so even without your fix callCount will never be anything other than 1.

          Andrew Shulman added a comment - Thanks for the quick fix. It's worth noting, though, that your test won't do exactly what you think it will. If you decompile the bridge method javap -v Derived void method(org.bukkit.event.Event); descriptor: (Lorg/bukkit/event/Event;)V flags: ACC_BRIDGE, ACC_SYNTHETIC Code: stack=2, locals=2, args_size=2 0: aload_0 1: aload_1 2: checkcast #2 // class org/bukkit/event/block/BlockPlaceEvent 5: invokevirtual #3 // Method method:(Lorg/bukkit/event/block/BlockPlaceEvent;)V 8: return LineNumberTable: line 2: 0 RuntimeVisibleAnnotations: 0: #13() All it does is call the overriding method, so even without your fix callCount will never be anything other than 1.

          Jonas Konrad added a comment -

          Fixed in bukkit PR #65.

          Jonas Konrad added a comment - Fixed in bukkit PR #65.

          SpigotMC added a comment -

          Your build is not the latest and therefore may be the reason you are having this issue. Spigot is 1 version(s) behind. CraftBukkit is 1 version(s) behind. This message was automatically generated and is not guaranteed to be a solution to your issue.

          SpigotMC added a comment - Your build is not the latest and therefore may be the reason you are having this issue. Spigot is 1 version(s) behind. CraftBukkit is 1 version(s) behind. This message was automatically generated and is not guaranteed to be a solution to your issue.

            Assignee:
            Unassigned
            Reporter:
            Andrew Shulman
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: