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

Fix SIGSEGV on file system deletion while building #765

Merged
merged 2 commits into from
Sep 25, 2019
Merged

Fix SIGSEGV on file system deletion while building #765

merged 2 commits into from
Sep 25, 2019

Conversation

Onlinehead
Copy link

Fix issue #756
That error happens only if you have files/directories inside a path marked as VOLUME
Whitelisting and delete functions don't expect you have something inside a VOLUME path and deleting content inside that dir (which is a correct behavior) breaks processing files inside the dir.
The fix adds isExists check for file/directory before info.IsDir() and skip path processing if it is not exist.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@Onlinehead
Copy link
Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@@ -137,6 +141,13 @@ func DeleteFilesystem() error {
return os.RemoveAll(path)
})
}
// isExists returns tru if path exists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: true

@@ -121,6 +121,10 @@ func DeleteFilesystem() error {
logrus.Info("Deleting filesystem...")
return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, _ error) error {
if CheckWhitelist(path) {
if ! isExist(path) {
logrus.Debugf("Path %s whitelisted, but not exists", path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: double space after Path

@@ -121,6 +121,10 @@ func DeleteFilesystem() error {
logrus.Info("Deleting filesystem...")
return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, _ error) error {
if CheckWhitelist(path) {
if ! isExist(path) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be run through gofmt to get travis to pass

@Rayyis
Copy link

Rayyis commented Sep 25, 2019

@Onlinehead Thanks for the fix. Is there a workaround until this gets merged? or is there something to do to speedup the process of releasing it? @jonjohnsonjr

@jonjohnsonjr jonjohnsonjr merged commit 51734fc into GoogleContainerTools:master Sep 25, 2019
@Onlinehead Onlinehead deleted the fix/filesystem-delete-fix branch December 5, 2019 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants