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

Write classes only once #2637

Closed
wants to merge 1 commit into from
Closed

Conversation

Rawi01
Copy link
Collaborator

@Rawi01 Rawi01 commented Nov 2, 2020

This PR fixes mplushnikov/lombok-intellij-plugin#969.

In
https://github.com/eclipse/eclipse.jdt.core/blob/b3cc25e26a9b327ef6e79fbec3cde4d4fbeb60fc/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompilerImpl.java#L444-L448
the stream gets closed twice. By wrapping the ByteArrayOutputStream in a FilterOutputStream the second close no longer perform any action.

@rzwitserloot After merging your latest Tycho changes I get a java.lang.IllegalArgumentException: argument type mismatch error. InteliJ does not print a full stacktrace so I do not know what exactly is broken.

@Rawi01 Rawi01 changed the title [bugfix] Write classes only once Write classes only once Nov 2, 2020
@mplushnikov
Copy link

@rspilker @rzwitserloot Can you please merge this pr?

@rzwitserloot
Copy link
Collaborator

As I understand it, you're wrapping the BAOS in a FilterOutputStream because the impl of FOS has a volatile boolean-based system to ensure a close() call is only propagated to the wrapped OutputStream exactly once.

Unfortunately, a quick check of the JDK sources shows that this is not how FOS worked in java7. The volatile boolean was introduced somewhere between 7 and 14.

Given that we are still targeting some fairly old versions, I think we need to handroll this boolean. Shouldn't be a problem, it's still only like 5 lines of code. I'll set it up tonight, just informing you that the thing this PR intends to fix will be fixed, but not via this PR.

Anything I missed?

@Rawi01
Copy link
Collaborator Author

Rawi01 commented Dec 3, 2020

@rzwitserloot That's exactly why I wrapped it in a FilterOutputStream. I have not checked the older versions because the current comment described that behaviour and I thought that it behaves like that since Java 1...

rzwitserloot added a commit that referenced this pull request Dec 4, 2020
…causing corrupt classfiles

Would crash with java.lang.ClassFormatError: Extra bytes at the end of class file de/lomboktest/Application
Fixes mplushnikov/lombok-intellij-plugin#969

figuring out the problem was the hard work - credits to @Rawi01 for discovering this
@mkurz
Copy link
Contributor

mkurz commented Dec 4, 2020

Any chance you update the edge release?
https://projectlombok.org/download-edge

@rzwitserloot
Copy link
Collaborator

@mkurz just released a new edge, with this fix.

@mkurz
Copy link
Contributor

mkurz commented Dec 4, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lombok + Eclipse compiler problem
4 participants