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

Some strange crash with chokidar watcher #86

Closed
Omikhleia opened this issue Aug 13, 2016 · 2 comments · Fixed by #88
Closed

Some strange crash with chokidar watcher #86

Omikhleia opened this issue Aug 13, 2016 · 2 comments · Fixed by #88

Comments

@Omikhleia
Copy link
Collaborator

Observed on my Windows box: someObject.destroy() sometimes (but neither systematically nor even frequently) causes an abrupt exit with an "uncaughtException" trapped in 'on-exit.js', due to an EPERM error. The stack trace doesn't tell much beyond that and doesn't tell which file/directory caused it, but after having added try/catch's around all fs stuff in fs-db.js, it was still occurring - It only stopped (so far as it seems at least) when I commented out the chokidar watcher. (Sure, that's a pretty mean workaround, lol.)

Tracked here for the record, in any case it would be observed by others, but no investigation required yet, the point is maybe moot:

  • It might be due to my modified fork, where I played with the directory structure and may have caused some havoc there (though I think I got the issue even before that. Will have to try to reproduce...)
  • Or it might be Windows-specific, so well... ;)
@Omikhleia
Copy link
Collaborator Author

That's a chokidar known issue, apparently:
paulmillr/chokidar#324 (open since Jul 2015)

Still reproduced after forcing the upgrade to 1.6.0 (roomjs was requiring 1.4.x).

@Omikhleia
Copy link
Collaborator Author

Omikhleia commented Aug 13, 2016

Ah, pinpointed it, apparently.
As far as I can say, seems avoidable with chokidar (1.6.0 at least), assuming the calling code declares a .on("error", ...) handler

Thus:

  • In fs-db.js, change this in setupWatcher():
    this.watcher.on('ready', () => {
      this.watcher
        .on('add', this.onFileAdded.bind(this))
        .on('change', this.onFileChanged.bind(this))
        .on('unlink', this.onFileRemoved.bind(this))
        // BEGIN PATCH
        .on('error', (error) => { 
            // Do some log here
        });
        // END PATCH
      this.emit('ready');
  • and also fix the rmDir() method to gracefully accept errors
  rmDir(relpath) {
    const filepath = this.toFilepath(relpath);
    // BEGIN PATCH
    try {
      fs.rmdirSync(filepath);  
    } catch (err) { 
      // Do some log, possibly...
      // At least two observed cases here:
      // - ENOTEMPTY
      // - ENOENT
    }    
    // END PATCH
  }

The second patch is somewhat unrelated, it's notably triggered when deleting a directory from outside the game (so there might be a loophole elsewhere, chokidar notifiying roomjs, and then the object deletion triggering another fs-db action?), but also if the directory is not empty (e.g. I had the bad idea to put a non-game file in a directory, in the form of a readme.md :) - So it's probably still a reasonable safeguard to have a try/catch here.)

So far, no more random crashing observed.

Omikhleia added a commit to Omikhleia/room.js that referenced this issue Aug 16, 2016
Has been running now for a while without showing crashes on object / deletion, whether from in-game destroy() or from the exterior (e.g. manual file removal). We get a warning log in both cases.
Omikhleia added a commit to Omikhleia/room.js that referenced this issue Aug 28, 2016
Omikhleia added a commit to Omikhleia/room.js that referenced this issue Sep 4, 2016
Has been running now for a while without showing crashes on object / deletion, whether from in-game destroy() or from the exterior (e.g. manual file removal). We get a warning log in both cases.
Omikhleia added a commit to Omikhleia/room.js that referenced this issue Sep 4, 2016
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

Successfully merging a pull request may close this issue.

1 participant