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

Fix delete of plugin directory on remove plugin #24266

Merged
merged 4 commits into from Apr 22, 2017

Conversation

Projects
None yet
3 participants
@jasontedor
Member

jasontedor commented Apr 22, 2017

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 #24252

Fix delete of plugin directory on remove plugin
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.

jasontedor added some commits Apr 22, 2017

@jasontedor jasontedor merged commit 9912650 into elastic:master Apr 22, 2017

1 of 2 checks passed

elasticsearch-ci Build started sha1 is merged.
Details
CLA Commit author is a member of Elasticsearch
Details

jasontedor added a commit that referenced this pull request Apr 22, 2017

Fix delete of plugin directory on remove plugin
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

jasontedor added a commit that referenced this pull request Apr 22, 2017

Fix delete of plugin directory on remove plugin
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

jasontedor added a commit that referenced this pull request Apr 22, 2017

Fix delete of plugin directory on remove plugin
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

@jasontedor jasontedor deleted the jasontedor:remove-plugin-fiasco branch Apr 22, 2017

asettouf added a commit to asettouf/elasticsearch that referenced this pull request Apr 23, 2017

Fix delete of plugin directory on remove plugin
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 elastic#24266

@clintongormley clintongormley added the >bug label Apr 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment