-
Notifications
You must be signed in to change notification settings - Fork 10
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
Update to file-leak-detector:1.11 to detect pipe and selector leaks #3
Changes from 9 commits
504dc08
a48cbca
4252593
03c4147
e3f1f5e
2e71e6d
8036e81
698ba37
d250a19
3cc9fdc
f92223d
a776c82
1604710
b325d60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,4 @@ | ||
target | ||
work | ||
.idea | ||
file-leak-detector.iml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
// Build the plugin using https://github.com/jenkins-infra/pipeline-library | ||
buildPlugin() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
<?jelly escape-by-default='true'?> | ||
<!-- | ||
This view is used to render the installed plugins page. | ||
--> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
/* | ||
* The MIT License | ||
* | ||
* Copyright 2018 CloudBees, Inc. | ||
* | ||
* Permission is hereby granted, free of charge, to any person obtaining a copy | ||
* of this software and associated documentation files (the "Software"), to deal | ||
* in the Software without restriction, including without limitation the rights | ||
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
* copies of the Software, and to permit persons to whom the Software is | ||
* furnished to do so, subject to the following conditions: | ||
* | ||
* The above copyright notice and this permission notice shall be included in | ||
* all copies or substantial portions of the Software. | ||
* | ||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
* THE SOFTWARE. | ||
*/ | ||
|
||
package com.cloudbees.jenkins.plugins.file_leak_detector; | ||
|
||
import com.gargoylesoftware.htmlunit.Page; | ||
import hudson.model.ManagementLink; | ||
import java.io.File; | ||
import java.io.FileInputStream; | ||
import java.io.InputStream; | ||
import java.nio.channels.Pipe; | ||
import java.nio.channels.Selector; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
import org.jvnet.hudson.test.JenkinsRule; | ||
import org.jvnet.hudson.test.JenkinsRule.WebClient; | ||
|
||
import static org.hamcrest.CoreMatchers.allOf; | ||
import static org.hamcrest.CoreMatchers.anyOf; | ||
import static org.hamcrest.CoreMatchers.containsString; | ||
import static org.hamcrest.CoreMatchers.hasItem; | ||
import static org.hamcrest.CoreMatchers.not; | ||
import static org.junit.Assert.assertThat; | ||
|
||
public class FileHandleDumpTest { | ||
|
||
@Rule | ||
public JenkinsRule r = new JenkinsRule(); | ||
|
||
@Rule | ||
public TemporaryFolder f = new TemporaryFolder(); | ||
|
||
@Test | ||
public void detectFileLeak() throws Exception { | ||
activateFileLeakDetector(); | ||
File leakyFile = f.newFile("leaky-file"); | ||
try (InputStream unused = new FileInputStream(leakyFile)) { | ||
assertThat("There should be a file opened by this method", getOpenDescriptors(), hasItem(allOf( | ||
containsString(leakyFile.getPath() + " by thread:"), | ||
containsString("FileHandleDumpTest.detectFileLeak(") | ||
))); | ||
} | ||
assertThat("There should not be a file opened by this method", getOpenDescriptors(), not(hasItem(allOf( | ||
containsString(leakyFile.getPath() + " by thread:"), | ||
containsString("FileHandleDumpTest.detectFileLeak(") | ||
)))); | ||
} | ||
|
||
@Test | ||
public void detectPipeLeak() throws Exception { | ||
activateFileLeakDetector(); | ||
Pipe p = null; | ||
try { | ||
p = Pipe.open(); | ||
assertThat("There should be a pipe sink and source channel opened by this method", getOpenDescriptors(), allOf( | ||
hasItem(allOf( | ||
containsString("Pipe Sink Channel by thread:"), | ||
containsString("FileHandleDumpTest.detectPipeLeak(") | ||
)), hasItem(allOf( | ||
containsString("Pipe Source Channel by thread:"), | ||
containsString("FileHandleDumpTest.detectPipeLeak(") | ||
)))); | ||
} finally { | ||
if (p != null) { | ||
p.sink().close(); | ||
p.source().close(); | ||
} | ||
} | ||
assertThat("There should not be a pipe sink or source channel opened by this method", getOpenDescriptors(), not(anyOf( | ||
hasItem(allOf( | ||
containsString("Pipe Sink Channel by thread:"), | ||
containsString("FileHandleDumpTest.detectPipeLeak(") | ||
)), hasItem(allOf( | ||
containsString("Pipe Source Channel by thread:"), | ||
containsString("FileHandleDumpTest.detectPipeLeak(") | ||
))))); | ||
} | ||
|
||
@Test | ||
public void detectSelectorLeak() throws Exception { | ||
activateFileLeakDetector(); | ||
try (Selector unused = Selector.open()) { | ||
assertThat("There should be a selector opened by this method", getOpenDescriptors(), hasItem(allOf( | ||
containsString("selector by thread:"), | ||
containsString("FileHandleDumpTest.detectSelectorLeak(") | ||
))); | ||
} | ||
assertThat("There should not be a selector opened by this method", getOpenDescriptors(), not(hasItem(allOf( | ||
containsString("selector by thread:"), | ||
containsString("FileHandleDumpTest.detectSelectorLeak(") | ||
)))); | ||
} | ||
|
||
/** | ||
* Activates the file-leak-detector library. Only activates once for the | ||
* whole test suite, but is called in every test so they can run in any | ||
* order, and cannot not be called until Jenkins starts. | ||
*/ | ||
private void activateFileLeakDetector() throws Exception { | ||
FileHandleDump fileHandleDump = ManagementLink.all().get(FileHandleDump.class); | ||
fileHandleDump.doActivate(""); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fails on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. I'll investigate to see whether we can fix it here or if the library itself doesn't work on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really have no idea. You can always buildPlugin(platforms: ['linux']) if it is known not to work on Windows. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating to JNR fixed the main issue, but pipes aren't detected correctly on Windows by the library, so I've ignored that test on Windows for now. See jenkinsci/lib-file-leak-detector#36. |
||
} | ||
|
||
/** | ||
* @return a list of open file detectors as detected by the file-leak-detector | ||
* library. Items in the list have the following format: | ||
* <pre>{@code | ||
* <descriptor type> by thread:<thread name> on <date descriptor was opened> | ||
* <stack trace of call that opened the descriptor> | ||
* }</pre> | ||
*/ | ||
private List<String> getOpenDescriptors() throws Exception { | ||
FileHandleDump fileHandleDump = ManagementLink.all().get(FileHandleDump.class); | ||
WebClient wc = r.createWebClient(); | ||
Page p = wc.goTo(fileHandleDump.getUrlName(), "text/plain"); | ||
String descriptorDump = p.getWebResponse().getContentAsString(); | ||
String[] descriptors = descriptorDump.split("#\\d+ "); | ||
// First element is similar to "6 descriptors are open", so we exclude it. | ||
return Arrays.asList(descriptors).subList(1, descriptors.length); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think idea editor specific files needs to be added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you added it originally in 504dc08 😄
I will go ahead and remove it, but for what it's worth
.idea
and other IntelliJ/Eclipse files are added to.gitignore
by default for any plugins using the maven archetype generator: https://github.com/jenkinsci/archetypes/blob/master/empty-plugin/src/main/resources/archetype-resources/.gitignore. We could follow up with a separate PR to add all of those entries to.gitignore
if desired.