quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

ManagedExecutorConfig does not seem to be working when using constructor injection

Open manofthepeace opened this issue 1 year ago • 4 comments

Describe the bug

when using something like this it works as expected;

public class App {

    private static final Logger LOGGER = Logger.getLogger("App");

    @Inject
    @ManagedExecutorConfig(maxAsync = 5)
    ManagedExecutor executor;

    void onStart(@Observes StartupEvent ev) {
        IntStream.range(0, 100).forEach(i -> executor.runAsync(() -> LOGGER.info(i)));
    }

}

but when changing to this

public class App {

    private static final Logger LOGGER = Logger.getLogger("App");

    ManagedExecutor executor;

    App(@ManagedExecutorConfig(maxAsync = 5) ManagedExecutor executor) {
        this.executor = executor;
    }

    void onStart(@Observes StartupEvent ev) {
        IntStream.range(0, 100).forEach(i -> executor.runAsync(() -> LOGGER.info(i)));
    }

}

The config is not respected

Expected behavior

Annotation could be disallowed on parameters or it should be otherwise working

Actual behavior

Annotation is not respected. in the first snippet the thread numbers stays low in the logs in the second variant it goes up to about 75.

How to Reproduce?

Reproducer: https://github.com/manofthepeace/executorconfig-ctorinject

1- mvn quarkus:dev 2- look at the thread number; [App] (executor-thread-76)

Output of uname -a or ver

Darwin Kernel Version 21.6.0

Output of java -version

Temurin-21.0.2+13

Quarkus version or git rev

3.7.3

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

maven 3.9.5

Additional information

No response

manofthepeace avatar Feb 16 '24 19:02 manofthepeace

/cc @Ladicek (arc), @manovotn (arc), @mkouba (arc)

quarkus-bot[bot] avatar Feb 16 '24 19:02 quarkus-bot[bot]

This should work and is likely a bug in the respective processor (https://github.com/quarkusio/quarkus/blob/main/extensions/smallrye-context-propagation/deployment/src/main/java/io/quarkus/smallrye/context/deployment/SmallRyeContextPropagationProcessor.java). I'll take a look on Mon.

manovotn avatar Feb 16 '24 21:02 manovotn

Hm, the processor here assumes that it operates on an AnnotationTarget that's METHOD_PARAMETER but instead, Arc gives it the whole METHOD. This doesn't seem very useful as you cannot really modify qualifiers of each method parameter which is what we'd need here.

Furthermore, this isn't the only place where we make such assumption, another processor doing the same is grpc processor and I'd assume this will also never work.

The Arc code seems very deliberate in not handling method parameters and it looks like something we encountered while integrating CDI BC extensions as well.

manovotn avatar Feb 17 '24 13:02 manovotn

I have a WIP draft fixing this in my branch but I need to take a closer look on Mon and polish it a little. Plus it looks like there are other pieces of code that might be using the IP transformer incorrectly which I'd like to double check as well.

manovotn avatar Feb 18 '24 13:02 manovotn