[JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. #3067

KostyaSha · 2017-10-09T08:41:06Z

PR for discussion.

mkdirs do remoting call by creating anonymous class.
Remoting on remote side must load outer FilePath class with imports.
FilePath had SystemProperties OnMaster.

@stephenc @oleg-nenashev i created and connected slave.jar via stdin/stdout and it fails with logically obvious error on FreeStyle job execution. Tested on 2.16, 2.60.

Started by user anonymous
Building remotely on socket-cd5f5e77df87 (docker-io) in workspace /home/jenkins/workspace/io-test
java.io.IOException: remote file operation failed: /home/jenkins/workspace/io-test at hudson.remoting.Channel@14e8a184:socket-cd5f5e77df87: java.io.IOException: Remote call on socket-cd5f5e77df87 failed
	at hudson.FilePath.act(FilePath.java:1014)
	at hudson.FilePath.act(FilePath.java:996)
	at hudson.FilePath.mkdirs(FilePath.java:1179)
	at hudson.model.AbstractProject.checkout(AbstractProject.java:1276)
	at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:604)
	at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:529)
	at hudson.model.Run.execute(Run.java:1720)
	at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
	at hudson.model.ResourceController.execute(ResourceController.java:98)
	at hudson.model.Executor.run(Executor.java:404)
Caused by: java.io.IOException: Remote call on socket-cd5f5e77df87 failed
	at hudson.remoting.Channel.call(Channel.java:805)
	at hudson.FilePath.act(FilePath.java:1007)
	... 10 more
Caused by: java.lang.LinkageError: Failed to load jenkins.util.SystemProperties
	at hudson.remoting.RemoteClassLoader.loadClassFile(RemoteClassLoader.java:377)
	at hudson.remoting.RemoteClassLoader.findClass(RemoteClassLoader.java:285)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at hudson.FilePath.<clinit>(FilePath.java:2385)
	at sun.misc.Unsafe.ensureClassInitialized(Native Method)
	at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
	at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:156)
	at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
	at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
	at java.lang.reflect.Field.getLong(Field.java:611)
	at java.io.ObjectStreamClass.getDeclaredSUID(ObjectStreamClass.java:1707)
	at java.io.ObjectStreamClass.access$700(ObjectStreamClass.java:72)
	at java.io.ObjectStreamClass$2.run(ObjectStreamClass.java:484)
	at java.io.ObjectStreamClass$2.run(ObjectStreamClass.java:472)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.io.ObjectStreamClass.<init>(ObjectStreamClass.java:472)
	at java.io.ObjectStreamClass.lookup(ObjectStreamClass.java:369)
	at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:598)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1843)
	at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1713)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2000)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1535)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2245)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2169)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2027)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1535)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:422)
	at hudson.remoting.UserRequest.deserialize(UserRequest.java:217)
	at hudson.remoting.UserRequest.perform(UserRequest.java:131)
	at hudson.remoting.UserRequest.perform(UserRequest.java:50)
	at hudson.remoting.Request$2.run(Request.java:332)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:748)
	at ......remote call to socket-cd5f5e77df87(Native Method)
	at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1433)
	at hudson.remoting.UserResponse.retrieve(UserRequest.java:253)
	at hudson.remoting.Channel.call(Channel.java:797)
	... 11 more
Caused by: java.lang.NoClassDefFoundError: javax/servlet/ServletContextListener
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:642)
	at hudson.remoting.RemoteClassLoader.loadClassFile(RemoteClassLoader.java:373)
	at hudson.remoting.RemoteClassLoader.findClass(RemoteClassLoader.java:285)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at hudson.FilePath.<clinit>(FilePath.java:2385)
	at sun.misc.Unsafe.ensureClassInitialized(Native Method)
	at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
	at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:156)
	at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
	at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
	at java.lang.reflect.Field.getLong(Field.java:611)
	at java.io.ObjectStreamClass.getDeclaredSUID(ObjectStreamClass.java:1707)
	at java.io.ObjectStreamClass.access$700(ObjectStreamClass.java:72)
	at java.io.ObjectStreamClass$2.run(ObjectStreamClass.java:484)
	at java.io.ObjectStreamClass$2.run(ObjectStreamClass.java:472)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.io.ObjectStreamClass.<init>(ObjectStreamClass.java:472)
	at java.io.ObjectStreamClass.lookup(ObjectStreamClass.java:369)
	at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:598)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1843)
	at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1713)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2000)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1535)
	at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2245)
	at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2169)
	at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2027)
	at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1535)
	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:422)
	at hudson.remoting.UserRequest.deserialize(UserRequest.java:217)
	at hudson.remoting.UserRequest.perform(UserRequest.java:131)
	at hudson.remoting.UserRequest.perform(UserRequest.java:50)
	at hudson.remoting.Request$2.run(Request.java:332)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.ClassNotFoundException: javax.servlet.ServletContextListener
	at hudson.remoting.RemoteClassLoader$ClassLoaderProxy.fetch(RemoteClassLoader.java:770)
	at sun.reflect.GeneratedMethodAccessor429.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.__invoke(DelegatingMethodAccessorImpl.java:43)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at hudson.remoting.RemoteInvocationHandler$RPCRequest.perform(RemoteInvocationHandler.java:887)
	at hudson.remoting.Request$2.run(Request.java:332)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
	at org.jenkinsci.remoting.CallableDecorator.call(CallableDecorator.java:18)
	at hudson.remoting.CallableDecoratorList$1.call(CallableDecoratorList.java:21)
	at jenkins.util.ContextResettingExecutorService$2.call(ContextResettingExecutorService.java:46)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
	at ......remote call to channel(Native Method)
	at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1433)
	at hudson.remoting.Request.call(Request.java:172)
	at hudson.remoting.RemoteInvocationHandler.invoke(RemoteInvocationHandler.java:255)
	at com.sun.proxy.$Proxy5.fetch(Unknown Source)
	at hudson.remoting.RemoteClassLoader.findClass(RemoteClassLoader.java:293)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	... 40 more
Finished: FAILURE

This change is

mkdirs do remoting call by creating anonymous class. Remoting on remote side must load outer FilePath class with imports. FilePath had SystemProperties OnMaster.

stephenc · 2017-10-09T08:56:44Z

Well FilePath to my understanding, must be loadable on the remote agent, so 👍 from me... but it does make the wider question, what to do about SystemProperties

oleg-nenashev · 2017-10-09T09:32:23Z

@stephenc @KostyaSha There are other calls, which may use SystemProperties. Actually the current implementation works well on the most of agents, but it is not the first report regarding this issue.

I have created #3065, which should resolve the issue with class loading.

oleg-nenashev · 2017-10-09T09:40:57Z

Likely 👍 in this case anyway. I see no reason in specifying this setting in web.xml

oleg-nenashev · 2017-10-09T09:42:40Z

JENKINS-46386 as a reference is probably fine if we want to do backporting

KostyaSha · 2017-10-09T15:12:34Z

@daniel-beck daniel-beck self-requested a review 3 hours ago :D

KostyaSha · 2017-10-09T15:14:55Z

Well FilePath to my understanding, must be loadable on the remote agent, so 👍 from me... but it does make the wider question, what to do about SystemProperties

I not sure but probably we can make FilePath not loadable on remote, that would require using callable in separate classes. I tried, but as usual nested classes has unclear dependencies to outside variables.

@stephenc @oleg-nenashev and second question is why ssh/jnlp doesn't fail? Should i force load this class somehow after channel building?

oleg-nenashev · 2017-10-09T15:17:14Z

Should i force load this class somehow after channel building?

Actually the classloading of this class should happen automatically if the class is not blacklisted by Remoting. That's why the method usually works fine on agents

KostyaSha · 2017-10-09T15:17:27Z

And why it wasn't covered with testcase already?

oleg-nenashev · 2017-10-09T16:49:33Z

@KostyaSha what exactly? FilePath or Remote servlet context usage?

KostyaSha · 2017-10-09T16:51:43Z

FreeStyle on remoting. AbstractBuild exists for a long time, i didn't expect to have such problems for such old/stable job type.

oleg-nenashev · 2017-10-09T17:02:57Z

Well, this issue happens only in a particular configuration. Otherwise we would have hundreds of issues in the bugtracker. From what I see none of existing JTH/ATH tests catches the issue, and there are lots of one using Remoting with classloading and FilePath. I was also unable to reproduce it locally.

If you provide a step-by-step reproduction guide, I will write a test for it

oleg-nenashev · 2017-10-13T10:29:50Z

If #3065 does not land in time, I will merge this guy towards the weekly

daniel-beck · 2017-10-13T11:18:23Z

@oleg-nenashev

I will merge this guy towards the weekly

Would you revert this change when merging #3065?

oleg-nenashev · 2017-10-13T11:22:31Z

@daniel-beck No strong opinion. I do not see much sense to contribute VALIDATE_ANT_FILE_MASK_BOUND through providers

Don't use onloadable things in remoting mkdirs calls.

1f476da

mkdirs do remoting call by creating anonymous class. Remoting on remote side must load outer FilePath class with imports. FilePath had SystemProperties OnMaster.

KostyaSha changed the title ~~Don't use onloadable things in remoting mkdirs calls.~~ Don't use unloadable things in remoting mkdirs calls. Oct 9, 2017

oleg-nenashev added the needs-more-reviews Complex change, which would benefit from more eyes label Oct 9, 2017

oleg-nenashev approved these changes Oct 9, 2017

View reviewed changes

daniel-beck self-requested a review October 9, 2017 11:55

oleg-nenashev changed the title ~~Don't use unloadable things in remoting mkdirs calls.~~ [JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. Oct 13, 2017

oleg-nenashev self-assigned this Oct 13, 2017

oleg-nenashev added ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback and removed needs-more-reviews Complex change, which would benefit from more eyes labels Oct 13, 2017

oleg-nenashev merged commit 27e5943 into jenkinsci:master Oct 15, 2017

jglick mentioned this pull request Oct 16, 2017

[JENKINS-46386] - Detach ServletContext logic to a pluggable Provider #3065

Closed

4 tasks

jglick mentioned this pull request Mar 21, 2018

[JENKINS-46386] Make SystemProperties safe to call from agent JVMs #3362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. #3067

[JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. #3067

KostyaSha commented Oct 9, 2017 •

edited

Loading

stephenc commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 13, 2017

daniel-beck commented Oct 13, 2017

oleg-nenashev commented Oct 13, 2017

[JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. #3067

[JENKINS-46386] - Don't use unloadable things in remoting mkdirs calls. #3067

Conversation

KostyaSha commented Oct 9, 2017 • edited Loading

stephenc commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

KostyaSha commented Oct 9, 2017

oleg-nenashev commented Oct 9, 2017

oleg-nenashev commented Oct 13, 2017

daniel-beck commented Oct 13, 2017

oleg-nenashev commented Oct 13, 2017

KostyaSha commented Oct 9, 2017 •

edited

Loading