Skip to content

Commit

Permalink
Fix delete of plugin directory on remove plugin
Browse files Browse the repository at this point in the history
This commit fixes an issue when deleting the plugin directory while
executing the remove plugin command. Namely, we take out a file
descriptor on the plugin directory to traverse its contents to obtain
the list of files to delete. We leaked this file descriptor. On
Unix-based filesystems, this is not a problem, deleting the plugin
directory deletes the plugin directory. On Windows though, a delete is
not executed until the last file descriptor is closed. Since we leaked
this file descriptor, the plugin was not actually deleted. This led to
test failures that tried to cleanup left behind temporary directories
but these test failures were just exposing this bug. This commit fixes
this issue by ensuring that we close the file descriptor to the plugin
directory when we are finished with it.

Relates #24266
  • Loading branch information
jasontedor committed Apr 22, 2017
1 parent 85463d4 commit d390dc4
Showing 1 changed file with 7 additions and 3 deletions.
Expand Up @@ -36,6 +36,8 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE;

Expand Down Expand Up @@ -110,7 +112,9 @@ void execute(final Terminal terminal, final String pluginName, final Environment
* Add the contents of the plugin directory before creating the marker file and adding it to the list of paths to be deleted so
* that the marker file is the last file to be deleted.
*/
Files.list(pluginDir).forEach(pluginPaths::add);
try (Stream<Path> paths = Files.list(pluginDir)) {
pluginPaths.addAll(paths.collect(Collectors.toList()));
}
try {
Files.createFile(removing);
} catch (final FileAlreadyExistsException e) {
Expand All @@ -122,9 +126,9 @@ void execute(final Terminal terminal, final String pluginName, final Environment
}
// now add the marker file
pluginPaths.add(removing);
// finally, add the plugin directory
pluginPaths.add(pluginDir);
IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()]));
// at this point, the plugin directory is empty and we can execute a simple directory removal
Files.delete(pluginDir);

/*
* We preserve the config files in case the user is upgrading the plugin, but we print a
Expand Down

0 comments on commit d390dc4

Please sign in to comment.