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

[JENKINS-47324] Use NIO in FilePath#isDirectory #3864

Closed
wants to merge 2 commits into
base: master
from

Conversation

3 participants
@dwnusbaum
Copy link
Member

dwnusbaum commented Jan 25, 2019

See JENKINS-47324.

This came up as a possible improvement to help diagnose JENKINS-55356. Needs a test case at the least, and some investigation to understand the cases that previously would have returned false that would now throw exceptions to understand the risk of the change.

From the File#isDirectory Javadoc:

Where it is required to distinguish an I/O exception from the case that the file is not a directory, or where several attributes of the same file are required at the same time, then the Files.readAttributes method may be used.

Known cases where the behavior changes between the old and new implementations:

  • If the file does not exist, the new code throws NoSuchFileException instead of returning false
    • This change seems likely to affect existing callers, maybe we handle it specially?

Proposed changelog entries

  • Internal: Allow callers of FilePath#isDirectory to differentiate between failures and non-directories by throwing exceptions in the case of failure.

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@reviewbybees

@jvz

jvz approved these changes Jan 25, 2019

Copy link
Member

jvz left a comment

Makes sense to me. I'd suggest Mockito for any tests here. :)

@dwnusbaum

This comment has been minimized.

Copy link
Member Author

dwnusbaum commented Jan 25, 2019

Tests failures such as jenkins.util.VirtualFileTest#readLink are definitely caused by the change, maybe we just need LinkOption.NOFOLLOW_LINKS, but needs to be investigated.

@dwnusbaum dwnusbaum added the needs-fix label Jan 25, 2019

@jvz

This comment has been minimized.

Copy link
Member

jvz commented Jan 25, 2019

After playing with that API a lot, if you do NOFOLLOW_LINKS to check if something is a directory, then you won't be able to visit symlinks to directories. At least that was my understanding.

@dwnusbaum

This comment has been minimized.

Copy link
Member Author

dwnusbaum commented Jan 25, 2019

Yeah after investigating it just looks like some VirtualFile methods need to be updated.

@dwnusbaum

This comment has been minimized.

Copy link
Member Author

dwnusbaum commented Jan 28, 2019

Tests are passing, but the build failed on Java 11 due to a CNFE:

I don't think it was caused by this change, so I'll check if it has already been reported and if not, open a new ticket. EDIT: It looks similar to JENKINS-38072, although I'm not sure why we'd see it intermittently on ci.jenkins.io unless since I'd assume that our agents are configured correctly with a JDK matching the master.

Stack trace
Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to ubuntu-jenkinsinfra-highmem8cac11
		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1741)
		at hudson.remoting.UserRequest$ExceptionResponse.retrieve(UserRequest.java:357)
		at hudson.remoting.Channel.call(Channel.java:955)
		at hudson.FilePath.act(FilePath.java:1072)
		at hudson.FilePath.act(FilePath.java:1061)
		at org.jenkinsci.plugins.gitclient.Git.getClient(Git.java:137)
		at hudson.plugins.git.GitSCM.createClient(GitSCM.java:821)
		at hudson.plugins.git.GitSCM.createClient(GitSCM.java:812)
		at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1180)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:120)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:90)
		at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:77)
		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:51)
		at hudson.security.ACL.impersonate(ACL.java:290)
		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:48)
		at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
		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)
java.lang.NoClassDefFoundError: Could not initialize class com.sun.proxy.$Proxy9
	at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:488)
	at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:1015)
	at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:1001)
	at hudson.remoting.RemoteInvocationHandler.wrap(RemoteInvocationHandler.java:167)
	at hudson.remoting.Channel.export(Channel.java:768)
	at hudson.remoting.Channel.export(Channel.java:731)
	at org.jenkinsci.plugins.gitclient.LegacyCompatibleGitAPIImpl.writeReplace(LegacyCompatibleGitAPIImpl.java:198)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:564)
	at java.io.ObjectStreamClass.invokeWriteReplace(ObjectStreamClass.java:1220)
	at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1137)
	at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:349)
	at hudson.remoting.UserRequest._serialize(UserRequest.java:264)
	at hudson.remoting.UserRequest.serialize(UserRequest.java:273)
	at hudson.remoting.UserRequest.perform(UserRequest.java:223)
	at hudson.remoting.UserRequest.perform(UserRequest.java:54)
	at hudson.remoting.Request$2.run(Request.java:369)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
	at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1135)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.lang.Thread.run(Thread.java:844)
Also:   	Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to ubuntu-jenkinsinfra-highmem8cac11
			at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1741)
			at hudson.remoting.UserRequest$ExceptionResponse.retrieve(UserRequest.java:357)
			at hudson.remoting.Channel.call(Channel.java:955)
			at hudson.FilePath.act(FilePath.java:1072)
			at hudson.FilePath.act(FilePath.java:1061)
			at org.jenkinsci.plugins.gitclient.Git.getClient(Git.java:137)
			at hudson.plugins.git.GitSCM.createClient(GitSCM.java:821)
			at hudson.plugins.git.GitSCM.createClient(GitSCM.java:812)
			at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1180)
			at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:120)
			at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:90)
			at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:77)
			at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:51)
			at hudson.security.ACL.impersonate(ACL.java:290)
			at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:48)
			at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
			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)
java.lang.NoClassDefFoundError: Could not initialize class com.sun.proxy.$Proxy9
		at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
		at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
		at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
		at java.lang.reflect.Constructor.newInstance(Constructor.java:488)
		at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:1015)
		at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:1001)
		at hudson.remoting.RemoteInvocationHandler.wrap(RemoteInvocationHandler.java:167)
		at hudson.remoting.Channel.export(Channel.java:768)
		at hudson.remoting.Channel.export(Channel.java:731)
		at org.jenkinsci.plugins.gitclient.LegacyCompatibleGitAPIImpl.writeReplace(LegacyCompatibleGitAPIImpl.java:198)
		at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
		at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
		at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.lang.reflect.Method.invoke(Method.java:564)
		at java.io.ObjectStreamClass.invokeWriteReplace(ObjectStreamClass.java:1220)
		at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1137)
		at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:349)
		at hudson.remoting.UserRequest._serialize(UserRequest.java:264)
		at hudson.remoting.UserRequest.serialize(UserRequest.java:273)
		at hudson.remoting.UserRequest.perform(UserRequest.java:223)
		at hudson.remoting.UserRequest.perform(UserRequest.java:54)
		at hudson.remoting.Request$2.run(Request.java:369)
		at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:72)
		at java.util.concurrent.FutureTask.run(FutureTask.java:264)
		at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1135)
		at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
		at java.lang.Thread.run(Thread.java:844)
	Caused: java.io.IOException: Remote call on ubuntu-jenkinsinfra-highmem8cac11 failed
Caused: java.io.IOException: Remote call on ubuntu-jenkinsinfra-highmem8cac11 failed
	at hudson.remoting.Channel.call(Channel.java:961)
	at hudson.FilePath.act(FilePath.java:1072)
	at hudson.FilePath.act(FilePath.java:1061)
	at org.jenkinsci.plugins.gitclient.Git.getClient(Git.java:137)
	at hudson.plugins.git.GitSCM.createClient(GitSCM.java:821)
	at hudson.plugins.git.GitSCM.createClient(GitSCM.java:812)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1180)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:120)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:90)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:77)
	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:51)
	at hudson.security.ACL.impersonate(ACL.java:290)
	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:48)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	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)
@dwnusbaum

This comment has been minimized.

Copy link
Member Author

dwnusbaum commented Feb 13, 2019

I'm going to close this for now until I have more time to look into File#isDirectory to understand what failure cases this could surface.

@dwnusbaum dwnusbaum closed this Feb 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.