Mocking jOOQ 3.15+ fails with "attempted to add a method" #1118
Comments
|
I've had a quick look, and though I don't think I can help (I don't have much time, and I haven't worked on any projects that use MockK in a long time), I will summarise in case it is useful.
The MockK log message, originates from MockK uses ByteBuddy for instantiating new subclasses, and that's where the This SO post seems to indicate that MockK is using Byte Buddy incorrectly https://stackoverflow.com/questions/40774684/, but it could be something else. I suspect that this is the MockK function that needs further inspection: I would encourage you, or anyone else facing this issue, to try poking around in the MockK code. Make a PR that adds your test from https://github.com/marcelstoer/jooq-mockk-test into |
|
Thank you @aSemy for the extra documentation. I had gone through all that code before filing this issue (that's how I found #1117 😄 ) but I missed to add pointers to it my description, sorry. Interesting SO reference. Confirming that an established library like MockK is using its core foundation incorrectly would be quite a thing I guess. |
|
I just found another such case in a different library. @lukehutch's classgraph library cannot be updated beyond 4.8.64 (from early 2020). Starting from 4.8.65 I am getting the dreaded "class redefinition failed: attempted to add a method" error. Since the exception is triggered in native code, I can't pinpoint which exact class causes this. However, I think I can limit the set of potential culprits to four classes - all in the same inheritance hierarchy. See below Yet, when I compare the changes that went into that release, I see nothing that would affect the class signatures in any way. --> I'm as clueless as before |
|
Btw, the unit test in #1319 demonstrates that the stacktrace is as follows. I started digging at @aSemy Side note, The other thing I discovered is that the call stack when running the test against jOOQ 3.14.16 (passes) is exactly identical to that of running against 3.19.14 (fails). Not sure though what this means, good or bad? Should it be different because in the later case the test class implements a pile of interfaces? |
|
Ok, a smoking gun at last - maybe. @Raibaz I'd need some guidance on this. I noticed that, contrary to its name, So, I changed the implementation such that it only returns super classes (and interfaces) like so: This fixes it for my use case but other tests started failing. |
|
Ah, this is interesting indeed! What are the tests that started failing? The way it looks, the correct behavior is that |
|
As this is something rather serious, I suspect that other code relies on I'll commit my fix to PR #1319 anyway and would appreciate if you could also take a look, thanks. |
|
@Raibaz I rebased my test branch onto master and spent a few more hours on this today. However, with my limited understanding of how MockK works this just leads nowhere, sorry. After all, I think As for the actual issue, I'm more puzzled then ever before. @Raibaz If you have a minute, I would very much appreciate if you could debug my tiny test in #1319 and see if you spot anything obvious. |
|
I'm a bit puzzled myself because that's the ByteBuddy part in MockK, that I'm not really familiar with. May be worth getting in touch with the ByteBuddy team to see if they have any pointers. |
|
@aSemy had already dug up an SO post that suggested MockK used ByteBuddy incorrectly. As I am not familiar with ByteBuddy either, I can't comment on that.
However, AFAIU that centered around the way subclasses are built. If you execute my test, it looks like it fails even before the subclassing step. So, this is maybe the wrong lead entirely. |
|
We're running into this as well, is there any additional information? |
|
Hm, we're running into some issues with this as well, specifically when we're mocking jOOQ-codegen based records since jOOQ 3.20. After digging into their changes a bit only one change is visible in the list of classes within This interface differs from the others that were already on the class being mocked in that it has no I have however not been able to reproduce anything similar when building up a minimal sample thusfar. I may dig into it a bit more tomorrow, as this prevents us from updating jOOQ atm, which would become an issue in the near future. |
|
@bdalenoord maybe it helps if you start with the test I added in #1319. |
|
@marcelstoer strange thing is that our project succeeds with jOOQ 3.19.20 😕 but maybe I can figure something out with your testcase indeed, thx for the pointer. |
|
@marcelstoer I've checked out your branch from #1319, and your test fails succesfully there. I've also added a test-case to simulate our situation: where instead of mocking a When I update jOOQ to 3.20.2 (the latest, the version that I was trying to update our project to), suddenly, both testcases fail. As I concluded yesterday, the only difference for Edit hmm. Simply excluding package-private interfaces from the ProxyMaker#addInterfaces does not fix the test, whilst in that case the exact same list of classes/interfaces is being considered within the JvmInlineInstrumentation as there was in 3.19. I'm losing leads and don't really have an idea how to continue investigation. |
|
First things first, do you get the same exception as in my documented case?
If so, can you debug to reproduce the same behavior I documented at #1118 (comment)? All classes considered modifiable, yet the retransformation then still fails?
Can we ask you to do that (contact user 'raphw')? I am certain it promises better results if they are contacted by the maintainer of a reputable project than by some random dudes. |
|
@marcelstoer the exception and top of the stacktrace are exactly the same for both the Table-based as well as the Record-based mock once I switch to jOOQ 3.20.2, and I can confirm that all classes are considered modifiable before attempting retransformation: |
|
Do we know which method it tries do add to which class and then fails? |
|
@sgerke-1L I don't, and as the error happens in native code I'm not sure how I could be able to figure that out, no way to inspect the runtime at that point. |
|
Digging around with Unified Logging I've got some insight into where the failure occurs. I've added Logging for the Record-based test-case (my reproduction that fails with 3.20 but succeeds with 3.19): which originates in https://github.com/openjdk/jdk/blob/1a8c8e07fee33861d348f7b41fea0e3fd5bbc0af/src/hotspot/share/prims/jvmtiRedefineClasses.cpp#L1190 But as to why would return false here, I'm not sure. The sample above was for my Record-based test-case, but a similar output is given for the TableImpl-based test: |
This is fantastic, thanks! I learned something today.
Hhhmm...so it fails for the (pity they log an error on INFO) |
|
Another hunch I have based on the error is the location of the My reproduction for the Record-type is only for version 3.20, where The |
|
Given enough examples a pattern might emerge. So far with have the jOOQ
I should probably add this to my test and run it with your logging JVM-arg. |
|
Great insights, thanks for finding the The problem seems to occur when a class is mocked that implements an interface with a default implementation. |
By running the tests with `-Xlog:redefine+class*=trace:stdout`, I gained an insight into which method was actually causing the error. The minimal testcase provided by sgerke-1L showed that this hunch was actually correct, the DefaultMethodTest simulates the issue entirely without the jOOQ dependency.
By running the tests with `-Xlog:redefine+class*=trace:stdout`, I gained an insight into which method was actually causing the error. The minimal testcase provided by sgerke-1L showed that this hunch was actually correct, the DefaultMethodTest simulates the issue entirely without the jOOQ dependency, showing that the default method is again the issue.
|
@sgerke-1L Indeed, with your minimal testcase the output is very similar: and it points to the |
|
@bdalenoord I've updated #1366 to include a fix. I've also asked a question in byte-buddy raphw/byte-buddy#1795 to further clarify if this is a bug in byte-buddy or not. |
|
The upstream discussion resulted in the discovery of another potential fix, which is to use Let me know if there is anything else I can help with, otherwise I'd see the ball in the maintainer's side of the field now. |
This issue is the continuation of the analysis started and documented at https://stackoverflow.com/q/76665966/131929. Also, this is very closely related to #255 and #477 which both became stale.
I tried to debug further but the exception stems from native JVM code. Hence, I suspect it requires someone intimately familiar with this project to look into it. Allow me to tag the top contributors @oleksiyp, @aSemy and @Raibaz.
Prerequisites
Expected Behavior
Mocking the jOOQ 3.15+
TableImplclass should work.Current Behavior
It fails with
Failure Information (for bugs)
Mocking the same class with jOOQ 3.14 works fine. The 3.15+ versions are different in that they also implement interfaces which seems to trip up MockK. With the earlier jOOQ versions MockK prints this trace log
It fails both on Java 11 and Java 17.
Steps to Reproduce
Check out the reproducer at https://github.com/marcelstoer/jooq-mockk-test and run
mvn test.The text was updated successfully, but these errors were encountered: