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.

Sign up for GitHub

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

Jump to bottom

Fix panic when reading and writing s.writer concurrently #4710

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Fix panic when reading and writing s.writer concurrently #4710

merged 2 commits into from
Apr 17, 2024

Conversation

polyrabbit
Copy link
Contributor 

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1ed3c80]

goroutine 3559509 [running]:
github.com/juicedata/juicefs/pkg/chunk.(*wSlice).ID(0x10100c00f1f1c20?)
        /home/miaochangxin/workspace/juicefs/pkg/chunk/cached_store.go:472
github.com/juicedata/juicefs/pkg/vfs.(*sliceWriter).prepareID(0xc00f330680, {0x3fbda18, 0xc0110dcf00}, 0x0)
        /home/miaochangxin/workspace/juicefs/pkg/vfs/writer.go:91 +0x282
created by github.com/juicedata/juicefs/pkg/vfs.(*sliceWriter).write in goroutine 1
        /home/miaochangxin/workspace/juicefs/pkg/vfs/writer.go:139 +0x149

jiefenghuang
jiefenghuang reviewed Apr 16, 2024
View reviewed changes
pkg/vfs/writer.go Outdated Show resolved Hide resolved
davies
davies reviewed Apr 16, 2024
View reviewed changes
pkg/vfs/writer.go Outdated
@@ -120,7 +120,9 @@ func (s *sliceWriter) flushData() {
s.writer.Abort()
s.err = syscall.EIO
}
s.chunk.file.Lock()
s.writer = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better to remove this line

@davies
Copy link
Contributor

davies commented Apr 16, 2024

This could be introduced by #4694, which will start multiple gorotuine to prepare id, we should fix that also.

@polyrabbit
Copy link
Contributor Author

Before #4694, I suppose it was also possible to spawn multiple goroutines to prepare one ID?

@davies
Copy link
Contributor

davies commented Apr 16, 2024

it may spawn 2 goroutines when it crosss the 4M border.

@davies davies merged commit 41b93ee into juicedata:main Apr 17, 2024
26 checks passed
SandyXSD pushed a commit that referenced this pull request Jun 19, 2024
@polyrabbit @davies @SandyXSD
Fix panic when reading and writing s.writer concurrently (#4710)
89986f8 
Signed-off-by: Changxin Miao <[email protected]>
Co-authored-by: Davies Liu <[email protected]>
SandyXSD pushed a commit that referenced this pull request Jun 20, 2024
@polyrabbit @davies @SandyXSD
Fix panic when reading and writing s.writer concurrently (#4710)
44b1e55 
Signed-off-by: Changxin Miao <[email protected]>
Co-authored-by: Davies Liu <[email protected]>
@polyrabbit polyrabbit deleted the s.writer-panic branch September 4, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiefenghuang jiefenghuang jiefenghuang left review comments

@davies davies davies left review comments

@SandyXSD SandyXSD SandyXSD approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@polyrabbit @davies @SandyXSD @jiefenghuang