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

[Bug]: Bytebuddy version update causes Invisible parameter type error #22749

Closed
kileys opened this issue Aug 16, 2022 · 11 comments
Closed

[Bug]: Bytebuddy version update causes Invisible parameter type error #22749

kileys opened this issue Aug 16, 2022 · 11 comments
Assignees
Labels
bug dependencies Pull requests that update a dependency file P1

Comments

@kileys
Copy link
Contributor

kileys commented Aug 16, 2022

What happened?

org.apache.beam.vendor.guava.v26_0_jre.com.google.common.util.concurrent.UncheckedExecutionException: java.lang.IllegalStateException: Invisible parameter type of SerializationTest$1 arg0 for public SerializationTest$1$DoFnInvoker(SerializationTest$1)

https://github.com/Talend/beam-samples/runs/7856722514?check_suite_focus=true

Issue Priority

Priority: 1

Issue Component

Component: dependencies

@kileys kileys added this to the 2.41.0 Release milestone Aug 16, 2022
@github-actions github-actions bot added dependencies Pull requests that update a dependency file P1 labels Aug 16, 2022
@kileys
Copy link
Contributor Author

kileys commented Aug 16, 2022

Unvendored and updated bytebuddy version in PR #17317

@kileys
Copy link
Contributor Author

kileys commented Aug 16, 2022

@cushon
cc: @lukecwik

@lukecwik
Copy link
Member

lukecwik commented Aug 17, 2022

Out of curiosity does the test pass if we are using bytebuddy 1.11.0 implying a backwards incompatible change with bytebuddy and not due to the unshading of bytebuddy?

@cushon
Copy link
Contributor

cushon commented Aug 17, 2022

I'm seeing it pass with bytebuddy 1.12.3 and earlier, and fail with bytebuddy 1.12.4 and later, regardless of unshading: raphw/byte-buddy#1301

So this is concerning, but it seems like it would have been an issue regardless of unshading when the dependency was upgraded.

@lukecwik
Copy link
Member

Thanks, linking the release thread where this was initially communicated for context around the issue: https://lists.apache.org/thread/zxtj1pr81l5nqqxsc4mjwnssflg8jkth

cushon added a commit to cushon/beam that referenced this issue Aug 17, 2022
@aromanenko-dev
Copy link
Contributor

I may confirm that I have the same results with different bytebuddy version on my side as @cushon provided above.

Does anyone have an idea why it doesn't fail with Beam CI jobs/tests?

@cushon
Copy link
Contributor

cushon commented Aug 18, 2022

Does anyone have an idea why it doesn't fail with Beam CI jobs/tests?

The bytebuddy regression involves handling of classes in the default package, and the failure from Talend/beam-samples involves a class in the default package: https://github.com/Talend/beam-samples/blob/74b5dfae2b2d1ac4e1272467e7d4584f852b0888/serializableTests/src/test/java/SerializationTest.java#L22. Is it possible the Beam CI isn't exercising transformation of classes in the default package?

@aromanenko-dev
Copy link
Contributor

Right, I don't think we have Java files with default package:

$ pwd
/Users/alexeyromanenko/dev/github/beam
$ grep -inrL "package" --include \*.java 
./sdks/java/testing/jpms-tests/src/main/java/module-info.java

(this found one should be fixed)

@kileys kileys removed this from the 2.41.0 Release milestone Aug 18, 2022
@kileys
Copy link
Contributor Author

kileys commented Aug 18, 2022

Removing the milestone since it's rolled back for the release

cushon added a commit to cushon/beam that referenced this issue Aug 22, 2022
This release includes the fix for the issue with visibility checks for
types in the default package that caused
apache#22749.
cushon added a commit to cushon/beam that referenced this issue Aug 25, 2022
This release includes the fix for the issue with visibility checks for
types in the default package that caused
apache#22749.
MarcoRob pushed a commit to MarcoRob/beam that referenced this issue Sep 5, 2022
@kennknowles
Copy link
Member

@aromanenko-dev @cushon is this now resolved? We first reverted for the release, then downgraded the unvendored bytebuddy on the master branch. Do we have confirmation that it is fixed?

@cushon
Copy link
Contributor

cushon commented Sep 13, 2022

It's fixed now: #22814 upgraded to the latest bytebuddy version, which includes a fix, and added a local regression test.

dedocibula pushed a commit to dedocibula/beam that referenced this issue Sep 15, 2022
dedocibula pushed a commit to dedocibula/beam that referenced this issue Sep 15, 2022
This release includes the fix for the issue with visibility checks for
types in the default package that caused
apache#22749.
kkdoon pushed a commit to twitter-forks/beam that referenced this issue Sep 29, 2022
kkdoon pushed a commit to twitter-forks/beam that referenced this issue Sep 29, 2022
This release includes the fix for the issue with visibility checks for
types in the default package that caused
apache#22749.
cushon added a commit to cushon/beam that referenced this issue Oct 17, 2022
cushon added a commit to cushon/beam that referenced this issue Oct 17, 2022
This release includes the fix for the issue with visibility checks for
types in the default package that caused
apache#22749.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug dependencies Pull requests that update a dependency file P1
Projects
None yet
Development

No branches or pull requests

5 participants