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

def-conversion sometimes breaks when AOT-compiled #68

Open
DerGuteMoritz opened this issue Jan 24, 2023 · 5 comments · Fixed by #69
Open

def-conversion sometimes breaks when AOT-compiled #68

DerGuteMoritz opened this issue Jan 24, 2023 · 5 comments · Fixed by #69

Comments

@DerGuteMoritz
Copy link
Collaborator

Repro: https://github.com/bevuta/byte-streams-def-conversion-aot-issue

Essentially the same issue as #34 but with tools.deps and tools.build instead of Leiningen. The resulting error is the same:

Execution error (ClassCastException) at clj_commons.byte_streams.graph.ConversionGraph/assoc_conversion (graph.clj:117).
class clj_commons.byte_streams.graph.Type cannot be cast to class clj_commons.byte_streams.graph.Type (clj_commons.byte_streams.graph.Type is in unnamed module of loader 'app'; clj_commons.byte_streams.graph.Type is in unnamed module of loader clojure.lang.DynamicClassLoader @650a1aff)

It's very likely caused by https://clojure.atlassian.net/browse/CLJ-1741, i.e. when first AOT-compiling a namespace which depends on another namespace which contains a def-conversion call, and then AOT-compiling that namespace itself.

DerGuteMoritz added a commit to bevuta/byte-streams that referenced this issue Jan 24, 2023
Since this function is usually not called at runtime, the reflection
performance penalty should be tolerable.

Fixes clj-commons#68
@KingMob
Copy link
Collaborator

KingMob commented Jul 7, 2023

Update for posterity: it's probably not the same as CLJ-1741, even if it's related. 1741 is a duplicate of of CLJ-1650, but that was fixed in Clojure 1.8, so the timing seems wrong.

@KingMob
Copy link
Collaborator

KingMob commented Jul 7, 2023

Re-opening, because the problem isn't really fixed, we just removed the type hints.

But it's interfering with Eastwood, CircleCI, and automatic deploys.

@KingMob KingMob reopened this Jul 7, 2023
@KingMob
Copy link
Collaborator

KingMob commented Aug 27, 2023

Update. Now it's flat-out breaking the new HTTP/2 code. What's going on?

@DerGuteMoritz
Copy link
Collaborator Author

Ouch! Do you have a repro case to look at somewhere?

@KingMob
Copy link
Collaborator

KingMob commented Aug 29, 2023

No, I think I was confused. I don't think it's AOT-related now.

I think the issue is loading byte-streams in Netty threads that use their own classloader. It gives me a ClassCastException even when Netty finds the class because class identity is tied to classloader.

It just looked a bit like the AOT errors we've gotten in the past.

Right now I'm biting the bullet and trying to decide how best to insert the clj loader into Netty threads. It's become more of an issue with the way ALPN and http2 multiplexing work; we have to do more work in Netty than we did for http1, and that runs a greater risk of needing the clj loader.

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 a pull request may close this issue.

2 participants