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.

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

Potential resource leak in unwatch() - seen in getWatched() #1275

Open
aicioara opened this issue Mar 12, 2023 · 4 comments
Open

Potential resource leak in unwatch() - seen in getWatched() #1275

aicioara opened this issue Mar 12, 2023 · 4 comments

Comments

@aicioara
Copy link

aicioara commented Mar 12, 2023

Describe the bug

Background: I have a long running process (days/months) that will dynamically add and remove dirs to watch. I use .add() and .unwatch() for the two operations and getWatched() to retrieve the list of dirst watched at any given moment.

Problem: .unwatch() does not remove the files from the list that getWatched() returns, but the files are indeed not matched anymore.

The problem is two fold:

  1. The list returned by getWatched() is not reliable
  2. The internal this._watched Map will continue to grow over the lifespan of the process

Versions:

  • Chokidar version: 3.5.3
  • Node version: v18.9.0
  • OS version: MacOS

To Reproduce:

const chokidar = require('chokidar');
const watcher = new chokidar.FSWatcher({}).on('all', (event, path) => {
    console.log(event, path);
});

let WATCHED_PATH = null
function changePath(path) {
    if (WATCHED_PATH) {
        watcher.unwatch(WATCHED_PATH)
    }
    WATCHED_PATH = path
    watcher.add(WATCHED_PATH)
}

setTimeout(() => changePath('/tmp/path1'), 100)
setTimeout(() => changePath('/tmp/path2'), 200)
setTimeout(() => console.log(watcher.getWatched()), 300)

Expected behavior

{
  '/tmp': [ 'path2' ],
  '/tmp/path2': [ '11', '12', '13' ]
}

Actual behavior

{
  '/tmp': [ 'path2' ],
  '/tmp/path1': [ '1', '2', '3' ],
  '/tmp/path2': [ '11', '12', '13' ]
}

Additional context

Again, the actual behavior of the watching (the important part) is correct, just the output of getWatched() is incorrect. I believe that happens because of the use of ignored paths.

chokidar/index.js

Lines 485 to 488 in 0f163b8

this._ignoredPaths.add(path);
if (this._watched.has(path)) {
this._ignoredPaths.add(path + SLASH_GLOBSTAR);
}

While I could technically ignore the output of getWatched(), or even better, patch the library with a

clean() {
  this._watched.clear()
}

My main concern is that the long running process may leak arbitrary resources (file watchers, etc) across the system.

@paulmillr
Copy link
Owner

Right. I think removing glob support altogether will solve this if v4 is released.

@aicioara
Copy link
Author

If I understand correctly, this means that a fix will only be available in v4, correct?

I am running this with disableGlobbing: true, depth: 0 options already, but that does not appear to fix it.

@paulmillr
Copy link
Owner

If I understand correctly, this means that a fix will only be available in v4, correct?

I don't focus much on Chokidar these days, so unless someone contributes a fix, yes.

@aicioara
Copy link
Author

Thank you for clarifying. At the moment, I am using the following workaround, but I may take a stab at it if I get the bandwidth. I am not sure what are the drawbacks of my approach or whether any resources leak.

const chokidar = require('chokidar');

function newFSWatcher() {
    return new chokidar.FSWatcher({}).on('all', (event, path) => {
        console.log(event, path);
    });
}

let watcher = newFSWatcher()

let WATCHED_PATH = null
async function changePath(path) {
    if (WATCHED_PATH) {
        await watcher.close()
        watcher = newFSWatcher()
    }
    WATCHED_PATH = path
    watcher.add(WATCHED_PATH)
}

setTimeout(() => changePath('/tmp/path1'), 100)
setTimeout(() => changePath('/tmp/path2'), 200)
setTimeout(() => console.log(watcher.getWatched()), 300)

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

No branches or pull requests

2 participants