Commit f331fb6
authored
Fix Jackson classloader issue while reading tokens from disk (#4994)
Jackson's class resolver, if not defined on its `TypeFactory`, first tries to use `Thread#getContextClassLoader` of the current thread to attempt to load the requested type. On failure it will then attempt to use its own classloader. Since by design the default classloader is not defined, the first load attempt will fail (since the context classloader is the root IDE classpath) and fall back to using the classloader that loaded Jackson.
Effectively, this means that if we want to avoid setting the type classloader for every Jackson `ObjectMapper` instance, we must always have Jackson in the plugin classloader, rather than using the version bundled by the IDE so that it can find the classes it needs.
At the moment we only include Jackson in plugin-core, but it introduces a question on what happens if a leaf plugin (i.e. plugin-amazonq/plugin-toolkit) loads Jackson through core. Will Jackson successfully resolve classes correctly? Inspecting the conetents of v3.20, when we fixed our last Jackson issue, the answer seems to be yes.
Interestingly, though this change brings our packaging back to v3.19, we do not seem to be facing the same deserialization issue on 2023.3 as last time (#4732)
> In Jackson 2.16.x, the Kotlin module uses reflection on the class annotations to determine if it should step in to deserialize a Kotlin class.
> However in Jackson 2.17.x, they switched to the standard isAnnotationPresent call, which ultimately performs an identity check on the annotation.
> In <241, the plugin classloader for the Q plugin INCORRECTLY loads the Kotlin metadata class from the Kotlin plugin, which results in isKotlinClass returning false (because the identity check fails if loaded from different classloaders) and then we get the cryptic cannot deserialize from Object value (no delegate- or property-based Creator) from the KotlinModule noop-ing.
And as always, unit testing of classloader issues is not possible at the moment because we are not set up for E2E tests and the classloader in headless tests differs from that of a running IDE.1 parent e3c47d7 commit f331fb6
File tree
3 files changed
+5
-13
lines changed- .changes/next-release
- buildSrc/src/main/kotlin
3 files changed
+5
-13
lines changedLines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
Lines changed: 0 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | 38 | | |
43 | 39 | | |
44 | 40 | | |
| |||
Lines changed: 1 addition & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
| 23 | + | |
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| |||
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | 68 | | |
77 | 69 | | |
78 | 70 | | |
| |||
0 commit comments