Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

use RW lock for the File's lock #43

Merged
merged 2 commits into from
Dec 19, 2018
Merged

use RW lock for the File's lock #43

merged 2 commits into from
Dec 19, 2018

Conversation

schomatis
Copy link
Contributor

Based on ipfs/kubo@feafd11 with the difference that in flushUp we keep the Lock() call effectively taking the lock for writing since we're replacing the current node's value (@Stebalien please validate this).

(Related to #32.)

This allows us to, e.g., get the size, etc. in parallel.
@schomatis schomatis added the kind/enhancement A net-new feature or improvement to an existing feature label Dec 19, 2018
@schomatis schomatis added this to the Review the MFS interface milestone Dec 19, 2018
@schomatis schomatis self-assigned this Dec 19, 2018
@ghost ghost added the status/in-progress In progress label Dec 19, 2018
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -118,8 +119,8 @@ func (fi *File) Open(flags int, sync bool) (FileDescriptor, error) {
// pretty much the same thing as here, we should at least call
// that function and wrap the `ErrNotUnixfs` with an MFS text.
func (fi *File) Size() (int64, error) {
fi.nodelk.Lock()
defer fi.nodelk.Unlock()
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't have to happen in this PR but, ideally, we'd call GetNode() here instead of holding the lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have a couple of TODOs to refactor that (including also adding a SetNode() method).

node := fi.node
fi.nodelk.Unlock()
Copy link
Member

Choose a reason for hiding this comment

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

Could also call GetNode here.

@schomatis schomatis merged commit e520d5a into master Dec 19, 2018
@ghost ghost removed the status/in-progress In progress label Dec 19, 2018
@schomatis schomatis deleted the feat/file/use-rw-lock branch December 19, 2018 17:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants