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

Improved warning in Native.loadByUrl #127

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Improved warning in Native.loadByUrl #127

merged 1 commit into from
Aug 7, 2024

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 25, 2024

I spent a lot of time investigating an error shown in the console of a permanent Windows (10) Jenkins agent

… org.jvnet.winp.Native loadByUrl: Failed to load DLL from static location
java.lang.UnsatisfiedLinkError: Native Library C:\…\jarCache\ED\winp.x64.BFFE30B3B50581290B1866EF8D48C609.dll already loaded in another classloader
       at java.base/java.lang.ClassLoader$NativeLibrary.loadLibrary(ClassLoader.java:2471)
       at java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2700)
       at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2630)
       at java.base/java.lang.Runtime.load0(Runtime.java:768)
       at java.base/java.lang.System.load(System.java:1837)
       at org.jvnet.winp.Native.loadDll(Native.java:253)
       at org.jvnet.winp.Native.loadByUrl(Native.java:173)
       at org.jvnet.winp.Native.load(Native.java:135)
       at org.jvnet.winp.Native.<clinit>(Native.java:102)
       at org.jvnet.winp.WinProcess.enableDebugPrivilege(WinProcess.java:230)
       at hudson.util.ProcessTree$Windows.<clinit>(ProcessTree.java:725)
       at hudson.util.ProcessTree.get(ProcessTree.java:462)
       at …

which I thought was the cause of some problems using the agent. In fact this was a red herring: WinP was loaded successfully from a temp file and ProcessTree worked. The UnsatisfiedLinkError turned out to be because the agent was being launched directly from a terminal window, rather than via WinSW, and so WinswSlaveRestarter was not active. If the agent connection is dropped and then reëstablished (for example after a controller restart), a new copy of WinP is loaded in a new RemoteClassLoader in the same JVM and it needs to use this fallback logic to work. Since nothing is necessarily broken, the stack trace was misleading.

It would of course be better to avoid native code in agents. Unfortunately I do not see a viable alternative to ProcessTree.Windows for now; ProcessHandle lacks any ability to look up environment variables, and in practice it shows few commands and no arguments.

@jglick jglick requested a review from a team August 7, 2024 14:50
Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a new copy of WinP is loaded in a new RemoteClassLoader in the same JVM

I think the loading of the library twice is questionable but this is just changing logging. (with many disconnections we will end up loading the library over and over - and as the exported functions are still the same and in the JVM I doubt that it is needed).
There is likely to be funkyness also if the library changes (ie it is updated on the Jenkins controller side and then we have 2 different incompatable versions loaded on the remote side. My gut says the JVM will use the functions exported first and then linkgage errors may occur..

But this is just a change to the logs so it won't make anything worse than it is today. (but I feel there is an issue to file here)

@jglick
Copy link
Member Author

jglick commented Aug 7, 2024

There is likely to be funkyness also if the library changes

Possibly. My testing was limited to a simple agent reconnection, which worked fine.

@basil basil merged commit f82ae14 into master Aug 7, 2024
14 checks passed
@basil basil deleted the jglick-patch-1 branch August 7, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants