snivilised / agenor

🌀 concurrent directory walker
MIT License
0 stars 0 forks source link

stackoverflow in tree root navigation #282

Closed plastikfan closed 4 weeks ago

plastikfan commented 4 weeks ago

Found when testing using venus:

task venus -- -path . -pattern "*|flac"

results in ....

    📂  .
    📂  .
    📂  .
    📂  .
    📂  .
    📂  .

infinite recursion ensues

plastikfan commented 4 weeks ago

This is an issue only for the relative file system. Here in navigator-agent.go, shows the problem:

func (n *navigatorAgent) travel(ctx context.Context,
    ns *navigationStatic,
    vapour inspection,
) (bool, error) {
    var (
        parent = vapour.Current()
    )

    for _, entry := range vapour.Entries() {
        path := filepath.Join(parent.Path, entry.Name()) // "."
        info, e := entry.Info()

        if progress, err := ns.mediator.impl.Traverse(
            ctx, ns, servant{
                node: core.New(
                    path,
                    entry,
                    info,
                    parent,
                    e,
                ),
            },

At the top level of navigation (parent.Path: '.'), inside the range loop over Entries, first iteration

So the problem is with the entry, the name property is coming out as "", so when we come to create the path for the child, it comes out as the same name as its parent: ".", which means when we go a traverse for the child, we effectively traverse the same as the parent, resulting in endless recursion.

The problem is, entry.Name() should never return an empty string.

plastikfan commented 4 weeks ago

I wonder if this could be fixed as part of providing a path calc (#271). Any fix we dream up for this is only applicable to relative file systems.

There is the possibility that this problem is caused by the way the virtual tree is created by nefilim.

plastikfan commented 4 weeks ago

An interesting development, adding the following test (navigator-vanilla_test.gp) that aims to reproduce the issue:

        FEntry(nil, Label(lab.Static.RetroWave), &lab.NaviTE{
            Given:         "universal: Path is Root",
            Relative:      ".",
            Subscription:  enums.SubscribeUniversal,
            Callback:      lab.UniversalCallback("ROOT-PATH"),
            ByPassMetrics: true,
        }),

does not reproduce the issue. Perhaps the problem is isolated to venus.

plastikfan commented 4 weeks ago

This is is caused by a rogue entry appearing in the read contents of a directory:

func DefaultReadEntriesHook(sys fs.ReadDirFS,
    dirname string,
) ([]fs.DirEntry, error) {
    contents, err := fs.ReadDir(sys, dirname)
    if err != nil {
        return nil, err
    }

    return lo.Filter(contents, func(item fs.DirEntry, _ int) bool {
        return item.Name() != ".DS_Store"
    }), nil
}

for some reason, the first entry contains an item whose name is empty.

Some evil black magic is occuring inside mapFS:

func (i *mapFileInfo) Name() string               { return path.Base(i.name) }

Base is implemented as:

// Base returns the last element of path.
// Trailing slashes are removed before extracting the last element.
// If the path is empty, Base returns ".".
// If the path consists entirely of slashes, Base returns "/".
func Base(path string) string {
    if path == "" {
        return "."
    }
    // Strip trailing slashes.
    for len(path) > 0 && path[len(path)-1] == '/' {
        path = path[0 : len(path)-1]
    }
    // Find the last element
    if i := bytealg.LastIndexByteString(path, '/'); i >= 0 {
        path = path[i+1:]
    }
    // If empty now, it had only slashes.
    if path == "" {
        return "/"
    }
    return path
}
plastikfan commented 4 weeks ago

Instead of applying the fix directly into, DefaultReadEntriesHook, we will provide an override hook in venus, to check if the name returned is "." and filter out these entries.