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

ManagedExecutorConfig does not seem to be working when using constructor injection #38825

Closed
manofthepeace opened this issue Feb 16, 2024 · 4 comments · Fixed by #38841
Closed
Assignees
Labels
area/arc Issue related to ARC (dependency injection) area/context-propagation kind/bug Something isn't working
Milestone

Comments

@manofthepeace
Copy link
Contributor

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 manofthepeace added the kind/bug Something isn't working label Feb 16, 2024
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Feb 16, 2024
Copy link

quarkus-bot bot commented Feb 16, 2024

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

@manovotn
Copy link
Contributor

@manovotn
Copy link
Contributor

manovotn commented Feb 17, 2024

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/context-propagation kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants