Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@ConsumeEvent annotated method is called twice per message when generic parent class has an abstract method for it #29585

Closed
benjaminrau opened this issue Nov 30, 2022 · 12 comments · Fixed by #29620
Labels
area/vertx kind/bug Something isn't working
Milestone

Comments

@benjaminrau
Copy link

benjaminrau commented Nov 30, 2022

Describe the bug

Having a class extending a class with generics which has an abstract method for the consumer method, this will result in the method annotated with @ConsumeEvent on the extending class to be called twice per eventbus message.

Expected behavior

Its expected that a method annotated with @ConsumeEvent is invoked only once per message.

  1. Remove public abstract void consume(@NonNull T event); from GreetingService
  2. Start the application
  3. You will notice that the consumer is invoked only once per message

Actual behavior

Having a class extending a class with generics which has an abstract method for the consumer method, this will result in the method annotated with @Consume on the extending class to be called twice per eventbus message.

  1. Start the application
  2. You will notice duplicated outputs from consumer like consuming HelloEvent on HelloService: ee73ab41-6b9a-465a-967c-d264046e0cc0

How to Reproduce?

Reproducer:

  1. Clone https://github.com/benjaminrau/genericdoublesconsumer
  2. Start the application
  3. See the duplicated output for same UUID (there should be only one)

Ive also prepared a test which shows that the consumer is registered twice in Vertx HandlerMap: https://github.com/benjaminrau/genericdoublesconsumer/blob/master/src/test/java/org/acme/EventBusTest.java

Output of uname -a or ver

Linux br-builders-nb 5.14.0-1052-oem #59-Ubuntu SMP Fri Sep 9 09:37:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17" 2021-09-14 OpenJDK Runtime Environment (build 17+35-2724) OpenJDK 64-Bit Server VM (build 17+35-2724, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.14.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

The log output clearly shows that the consumer is called twice for the same message since the payload is a randomized UUID string.

2022-11-30 17:39:41,659 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-1) consuming HelloEvent on HelloService: cdd635b3-f7ee-4a07-adea-56fbebfc5366
2022-11-30 17:39:41,659 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: cdd635b3-f7ee-4a07-adea-56fbebfc5366
2022-11-30 17:39:43,659 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-1) consuming HelloEvent on HelloService: eedb7070-d1e8-4686-8e53-c6733926ad52
2022-11-30 17:39:43,659 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: eedb7070-d1e8-4686-8e53-c6733926ad52
@benjaminrau benjaminrau added the kind/bug Something isn't working label Nov 30, 2022
@geoand
Copy link
Contributor

geoand commented Dec 1, 2022

cc @mkouba

@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2022

Hm, it has to be something related to the lombok.NonNull annotation. I'm not able to reproduce the problem when I remove the annotation.

Actually, I'm not able to reproduce the problem in the development mode even with NonNull present. I can see the following output:

INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, smallrye-context-propagation, vertx]
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: a52751db-6ecf-48f7-9abc-25509f18f84c
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: cbbae845-8244-4c16-a5a8-c407191f5309
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: ea301ef3-f95c-4bc2-aaaf-f481e50a009e
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: 36cc81b5-d6d1-4e13-8b3a-27993da280df
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: 675e734d-5b56-49f0-8518-46236887b27b
INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: d60ac206-23b7-4ef0-9b3d-a78d89dca4a6

@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2022

@benjaminrau Hm, but the test failed and I can reproduce the issue in the production mode. Interesting.

@benjaminrau
Copy link
Author

benjaminrau commented Dec 1, 2022

@mkouba Maybe i understand you wrong but if you wonder why the test fails that might be probably misleading by me: I made the test fail until the bug is fixed. When the bug is fixed it should pass (only one consumer on the address is the expected bahaviour)

Ah, missed the first comment of yours. Actually i didnt check the issue for explicit mode, so i didnt notice there is no issue in dev mode.

Does NonNull make any difference at all? I will check that right now. It was just there from another (the last) reproduces if built few days ago.

@benjaminrau
Copy link
Author

benjaminrau commented Dec 1, 2022

I justed tested it and NonNull makes no difference here to me. The issues persists. Ive updated the reproducer (removed NonNull) to not be misleading there.

Furthermore i am not sure if you mean by development mode anything else that what the log states - if no, the issue is there for me on dev too:

2022-12-01 15:57:26,075 INFO  [io.quarkus] (Quarkus Main Thread) nonnullissue 1.0.0-SNAPSHOT on JVM (powered by Quarkus 2.14.2.Final) started in 0.949s. 
2022-12-01 15:57:26,079 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2022-12-01 15:57:26,079 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, smallrye-context-propagation, vertx]
2022-12-01 15:57:26,085 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: 0c56efbb-7faf-40d9-b482-b315e06a9dc0
2022-12-01 15:57:26,085 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-1) consuming HelloEvent on HelloService: 0c56efbb-7faf-40d9-b482-b315e06a9dc0
2022-12-01 15:57:28,075 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-0) consuming HelloEvent on HelloService: ebf67ce6-5eb6-4ef8-9a2b-ce01bc965f29
2022-12-01 15:57:28,075 INFO  [org.acm.HelloService] (vert.x-eventloop-thread-1) consuming HelloEvent on HelloService: ebf67ce6-5eb6-4ef8-9a2b-ce01bc965f29

@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2022

@benjaminrau #29620 should fix the problem. It would be great if you could give it a try.

@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2022

Actually i didnt check the issue for explicit mode, so i didnt notice there is no issue in dev mode.

It seems that the compiler used in the dev mode does not copy the annotations from the original method to the generated bridge method.

@benjaminrau
Copy link
Author

benjaminrau commented Dec 1, 2022

@mkouba I'd really love to but i have to admit that i dont know how to include quarkus at the given commit hash on my repo. Is there probably some documentation you could refer to how to do so?

@mkouba
Copy link
Contributor

mkouba commented Dec 1, 2022

@mkouba I'd really love to but i have to admit that i dont know how to include quarkus at the given commit hash on my repo. Is there probably some documentation you could refer to how to do so?

Oh, sorry. There are some tips in the readme. You would need to clone the project, then checkout the pull request branch, then build with something like ./mvnw clean install -Dquickly -T 7 and then in your pom.xml change the version to 999-SNAPSHOT and quarkus.platform.group-id to io.quarkus. Quite a lot of steps, right :-)

@benjaminrau
Copy link
Author

@mkouba Will try later today, will let you know whether i was successful or not. Thanks for the hints! Looks quite interesting!

@benjaminrau
Copy link
Author

benjaminrau commented Dec 1, 2022

@mkouba Following the steps i cant reproduce the issue any longer :) It seems you are fixing bugs faster than i am able to describe them. Incredible! Wants again you (Quarkus team) leave me stunned about the experience when reporting (possible) issues here on github.

Offtopic:
When i did mvnw -Dquickly it seemed to have failed with a lot of error messages like below but when i was stuck trying to fix that build i continued by doing the changes on the pom.xml and wondered that somehow my reproducer was anyway using the quarkus version from the pull request branch.

Is there an easy explanation for that?

[ERROR] /home/benjaminrau/Projects/quarkus-fix/extensions/redis-client/runtime/src/main/java/io/quarkus/redis/datasource/sortedset/SortedSetCommands.java:[182,47] Possible heap pollution from parameterized vararg type K

@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 2, 2022
@mkouba
Copy link
Contributor

mkouba commented Dec 2, 2022

Offtopic: When i did mvnw -Dquickly it seemed to have failed with a lot of error messages like below but when i was stuck trying to fix that build i continued by doing the changes on the pom.xml and wondered that somehow my reproducer was anyway using the quarkus version from the pull request branch.

Is there an easy explanation for that?

Hm, I thought that "Possible heap pollution" is a compiler warning... not an error.

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 2, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 2, 2022
@gsmet gsmet modified the milestones: 2.15.0.Final, 2.14.3.Final Dec 2, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 2, 2022
@gsmet gsmet modified the milestones: 2.14.3.Final, 2.13.6.Final Dec 14, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertx kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants