line / armeria

Your go-to microservice framework for any situation, from the creator of Netty et al. You can build any type of microservice leveraging your favorite technologies, including gRPC, Thrift, Kotlin, Retrofit, Reactive Streams, Spring Boot and Dropwizard.
https://armeria.dev
Apache License 2.0
4.8k stars 912 forks source link

Fix to call the duplicator's child stream subscriber methods by the correct executor #5783

Closed minwoox closed 3 months ago

minwoox commented 3 months ago

Motivation: Subscriber methods must be called by the executor that is specified when subscribing to a StreamMessage. However, the methods of the duplicator's child stream subscriber are currently being called from the duplicator's executor, which is incorrect.

Modifications:

Result:

github-actions[bot] commented 3 months ago

๐Ÿ” Build Scanยฎ (commit: 2d7fd3671c76b39a63478330169db32ad9e8b663)

Job name Status Build Scanยฎ
build-windows-latest-jdk-21 โœ… https://ge.armeria.dev/s/sgelch2c6grp4
build-self-hosted-unsafe-jdk-8 โœ… https://ge.armeria.dev/s/wpsz4e527sxae
build-self-hosted-unsafe-jdk-21-snapshot-blockhound โœ… https://ge.armeria.dev/s/d2xb5wnax3qn6
build-self-hosted-unsafe-jdk-17-min-java-17-coverage โœ… https://ge.armeria.dev/s/ngnpcqu3qe5os
build-self-hosted-unsafe-jdk-17-min-java-11 โœ… https://ge.armeria.dev/s/frf5w6sqxixq6
build-self-hosted-unsafe-jdk-17-leak โœ… https://ge.armeria.dev/s/hasiyzm6t6dxg
build-self-hosted-unsafe-jdk-11 โœ… https://ge.armeria.dev/s/fm2z4qwwei7au
build-macos-12-jdk-21 โœ… https://ge.armeria.dev/s/ge2okoorkgczw
minwoox commented 3 months ago

Static analysis was not easy because two executors were used together in StreamMessageProcessor. How about refactoring the implementation later to delegate the logic to DownstreamSubscription if DownstreamSubscription.executor is needed?

That's a good suggestion. ๐Ÿ‘ Addressed it from this PR. :wink:

minwoox commented 3 months ago

Thanks for the quick review. ๐Ÿ™‡ I'll test it with the snapshot version to ensure the problem is fixed.