Repository navigation
Is NewVFS intended to be constructed only once per process? #7690
|
At main 6aa2332, NewVFS mutates the package-level internalNodes slice and meta.TrashName. Using the existing pkg/vfs createTestVFS helper with memkv and in-memory object storage, a focused Linux Go 1.25.10 race run shows:
The first behavior comes from unconditionally removing the slice's last element. The second comes from prefixing names already modified by an earlier call: Lines 1266 to 1278 in 6aa2332 Is NewVFS intended to be initialized only once per process? The normal mount path appears to call it once, so this is not a report of a reproduced mounted-client failure or data loss. If repeated calls are supported, should these nodes be instance-local? A partial reset during construction would still affect earlier VFS instances and exported package-level lookup helpers. I can contribute the native regression and a scoped fix once the intended lifetime contract is clear. The reproduction used private temporary state and no FUSE mount or external metadata service. Native characterization test (pkg/vfs)Save as /*
* JuiceFS, Copyright 2026 Juicedata, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package vfs
import (
"path/filepath"
"testing"
"github.com/juicedata/juicefs/pkg/meta"
)
// This serial characterization uses the real VFS constructor and memkv store.
// Restore globals so the probe cannot change unrelated tests' starting state.
func TestOwnerVFSConstructionIsRepeatable(t *testing.T) {
initial := append([]*internalNode(nil), internalNodes...)
for i, node := range initial {
copyNode, copyAttr := *node, *node.attr
copyNode.attr = ©Attr
initial[i] = ©Node
}
trashName := meta.TrashName
restore := func() {
internalNodes = make([]*internalNode, len(initial))
for i, node := range initial {
copyNode, copyAttr := *node, *node.attr
copyNode.attr = ©Attr
internalNodes[i] = ©Node
}
meta.TrashName = trashName
}
t.Cleanup(restore)
// Never let NewVFS rename a process-global /tmp/state<parent>.json.
t.Setenv("_FUSE_STATE_PATH", filepath.Join(t.TempDir(), "absent-state.json"))
t.Run("default_control", func(t *testing.T) {
restore()
for i := 0; i < 2; i++ {
v, _ := createTestVFS(nil, "")
if entry, errno := v.Lookup(NewLogContext(meta.Background()), meta.RootInode, ".config"); errno != 0 || entry.Inode != ConfigInode {
t.Fatalf("construction %d: .config lookup = %v, %v", i+1, entry, errno)
}
}
})
t.Run("subdir_control_then_second_constructor", func(t *testing.T) {
restore()
for i := 0; i < 2; i++ {
v, _ := createTestVFS(func(conf *meta.Config) { conf.Subdir = "sub" }, "")
if node := getInternalNode(trashInode); node != nil {
t.Fatal("subdir constructor left the trash node visible")
}
if len(internalNodes) != len(initial)-1 {
t.Errorf("construction %d: %d internal nodes, want %d (only trash omitted)", i+1, len(internalNodes), len(initial)-1)
}
if entry, errno := v.Lookup(NewLogContext(meta.Background()), meta.RootInode, ".config"); errno != 0 || entry.Inode != ConfigInode {
t.Errorf("construction %d: .config lookup = %v, %v", i+1, entry, errno)
}
}
})
t.Run("prefix_control_then_second_constructor", func(t *testing.T) {
restore()
v, _ := createTestVFS(nil, "")
conf := *v.Conf
conf.PrefixInternal = true
for i := 0; i < 2; i++ {
prefixed := NewVFS(&conf, v.Meta, v.Store, nil, nil)
if entry, errno := prefixed.Lookup(NewLogContext(meta.Background()), meta.RootInode, ".jfs.config"); errno != 0 || entry.Inode != ConfigInode {
t.Errorf("construction %d: .jfs.config lookup = %v, %v", i+1, entry, errno)
}
if meta.TrashName != ".jfs"+trashName {
t.Errorf("construction %d: TrashName = %q, want %q", i+1, meta.TrashName, ".jfs"+trashName)
}
}
})
}AI-assisted investigation; the reported outcomes were independently reproduced with the native constructor. |
Replies: 1 comment
|
In practice yes: the only caller is |
In practice yes: the only caller is
juicefs mount(cmd/mount.go#L689), andpkg/vfskeeps more process-wide state thaninternalNodes, like the package-level op metrics thatvfs.InitMetricsregisters once and the/tmp/state<ppid>.jsonhandle file that everyNewVFScall loads and renames. Everything that may open several volumes in one process, such as the Java SDK'sjfs_init, goes throughfs.NewFileSysteminstead, which builds its own reader, writer and metrics per instance and never touches those globals. So for more than one instancepkg/fsis the route the code already supports. MakingNewVFSitself repeatable would mean moving the internal node table ontoVFS, andpkg/fsreads that tabl…