Skip to content

Fixing the creation of multiple inode creation for the same name when…#572

Open
skuppa-veeva wants to merge 1 commit intokahing:masterfrom
skuppa:prevent-duplicate-inode-creation
Open

Fixing the creation of multiple inode creation for the same name when…#572
skuppa-veeva wants to merge 1 commit intokahing:masterfrom
skuppa:prevent-duplicate-inode-creation

Conversation

@skuppa-veeva
Copy link
Copy Markdown

Fixing the creation of multiple inode creation for the same name when MkDir executed simultaneously.

… MkDir or CreateFile executed simultaneously.

Get parent lock before creating inode from `Goofys.MkDir` or `Goofys.CreateFile` to avoid creating a duplicate inode.
Remove get lock from `Inode.Create` and `Inode.MkDir` since it lock already acquired by the caller.
@skuppa skuppa force-pushed the prevent-duplicate-inode-creation branch from d714e44 to 1ff2c07 Compare October 27, 2020 18:54
Comment thread internal/dir.go
Comment on lines -908 to -910
parent.mu.Lock()
defer parent.mu.Unlock()

Copy link
Copy Markdown
Author

@skuppa-veeva skuppa-veeva Oct 28, 2020

Choose a reason for hiding this comment

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

We don't need to parent lock since it is already locked from caller.

Comment thread internal/dir.go
Comment on lines -951 to -953
parent.mu.Lock()
defer parent.mu.Unlock()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We need lock here as well, since caller already has a lock.

Comment thread internal/goofys.go
inode, fh := parent.Create(op.Name, op.Metadata)

parent.mu.Lock()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The folder creation is moved after acquiring the parent lock. This lock needs to be atomic between creating inode and inserting into map of inodes.

Comment thread internal/goofys.go
Comment on lines -1058 to -1059
parent.mu.Lock()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The folder creation is moved after acquiring the parent lock. This lock needs to be atomic between creating inode and inserting into map of inodes.

@skuppa-veeva
Copy link
Copy Markdown
Author

skuppa-veeva commented Oct 28, 2020

@kahing This PR fixes multiple issues:
Fix #353 #377 #571

Could you review and make sure the order of lock is correct and it works as expected. We are waiting for this fix and I would like to get you feedback.

@skuppa-veeva
Copy link
Copy Markdown
Author

@kahing could you review this PR and give us your expert advice.

@skuppa-veeva
Copy link
Copy Markdown
Author

@kahing could you review this PR?

@monthonk
Copy link
Copy Markdown
Contributor

monthonk commented Aug 8, 2022

This PR seems to fix multiple issues but we need a rebase to make it work with current code base. I'm not sure you're still tracking it but we probably want to try to get this PR merged as part of the next release.

@94jskim
Copy link
Copy Markdown

94jskim commented Apr 18, 2023

@kahing @gaul @monthonk I need this fix, when will it be released?

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.

3 participants