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

Dev mode - fix instrumentation based reload #14363

Merged
merged 1 commit into from Jan 19, 2021

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Jan 18, 2021

  • currently, a new member value added to an annotation does not trigger
    application restart

- currently, a new member added to an annotation does not trigger
application restart
@ghost ghost added the area/core label Jan 18, 2021
@gsmet
Copy link
Member

gsmet commented Jan 18, 2021

@mkouba not sure it's related but I just had an issue reloading an app that seems to be related to the same area:

2021-01-18 14:06:14,557 ERROR [io.qua.dep.dev.RuntimeUpdatesProcessor] (vert.x-worker-thread-4) Failed to replace classes via instrumentation: java.lang.IllegalStateException: Duplicate key javax.validation.constraints.NotNull (attempted merging values @NotNull and @NotNull)
    at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:133)
    at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:180)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
    at io.quarkus.deployment.dev.ClassComparisonUtil.compareAnnotations(ClassComparisonUtil.java:106)
    at io.quarkus.deployment.dev.ClassComparisonUtil.isSameStructure(ClassComparisonUtil.java:73)
    at io.quarkus.deployment.dev.RuntimeUpdatesProcessor.doScan(RuntimeUpdatesProcessor.java:211)
    at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$2.handle(VertxHttpHotReplacementSetup.java:62)
    at io.quarkus.vertx.http.runtime.devmode.VertxHttpHotReplacementSetup$2.handle(VertxHttpHotReplacementSetup.java:52)
    at io.vertx.core.impl.ContextImpl.lambda$executeBlocking$2(ContextImpl.java:313)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Thread.java:834)

@gsmet
Copy link
Member

gsmet commented Jan 18, 2021

I think it happens when you have several times the same annotation on different parameters of the same method as:

        Map<DotName, AnnotationInstance> lookup = b.stream()
                .collect(Collectors.toMap(AnnotationInstance::name, Function.identity()));

is not handling conflicts.

@gsmet
Copy link
Member

gsmet commented Jan 18, 2021

AFAICS, I can fix it without creating conflicts with what you did so I will push another PR.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no perfect, you fixed my issue too!

@mkouba
Copy link
Contributor Author

mkouba commented Jan 18, 2021

Unfortunately, it seems that ClassChangeAgent.instrumentation is never set for QuarkusDevModeTest and so I don't know how to write a test for this...

@mkouba mkouba marked this pull request as ready for review January 18, 2021 14:36
@stuartwdouglas stuartwdouglas merged commit efae7a4 into quarkusio:master Jan 19, 2021
@ghost ghost added this to the 1.12 - master milestone Jan 19, 2021
@gsmet gsmet modified the milestones: 1.12 - master, 1.11.1.Final Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants