garden / tree

A multiplayer file system
https://thefiletree.com
European Union Public License 1.2
70 stars 23 forks source link

fs.js: undefined this.driver crashes server #158

Closed jankeromnes closed 11 years ago

jankeromnes commented 12 years ago

Just to let you know that the server was down a long time, and the reason it crashed (and still crashes now) is this bug:

/home/dom/tree/lib/fs.js:201
    this.driver.read(this.path, function(err, data) {
                ^
TypeError: Cannot call method 'read' of undefined
    at File.open (/home/dom/tree/lib/fs.js:201:17)
    at /home/dom/tree/lib/fsapi.js:26:12
    at Object.file (/home/dom/tree/lib/fs.js:108:5)
    at read (/home/dom/tree/lib/fsapi.js:19:6)
    at EventEmitter.fsOperation (/home/dom/tree/lib/fsapi.js:108:7)
    at EventEmitter.emit (events.js:115:20)
    at IncomingMessage.gotrequest (/home/dom/tree/node_modules/camp/lib/camp.js:188:12)
    at IncomingMessage.EventEmitter.emit (events.js:88:17)
    at IncomingMessage._emitData (http.js:359:10)
    at HTTPParser.parserOnBody [as onBody] (http.js:123:21)

Hope we can fix it soon :)

jankeromnes commented 12 years ago

Haha, I actually reported this 1 month ago in #149. Closing the old one as duplicate.

bnjbvr commented 11 years ago

Hi!

I could reproduce the issue this way:

After some debugging, the file which generates the issue is "/". The bug appears because: 1) file.meta.type is undefined, and hence not dir (lib/fsapi.js, line 27) 2) the file "/" is in the cache fileFromPath (lib/fs.js, line 101) 3) but the cached version doesn't match the first version.

The first version of the file "/" in the cache is indeed correct and is composed with all the required fields. After some times, the cached version of the file "/" changes and some fields are missing.

The file "/" is actually removed from the cache when close() in fs.js is called a sufficient amount of times. It seems legit that this file should remain opened, even if there's nobody watching it.

This solution is more a hack than a real solution, but it fixes at least the bug I am experiencing and which is described above.

This bug hunt tells us that the file "/" is correctly opened the first times but then, something wrong happens and it's not anymore...

Good luck!

espadrine commented 11 years ago

@BenjBouv I can't seem to reproduce this, but your description of the conditions under which the bug occurs is quite interesting.

The only things I know that mutate file.meta are file::close, which remove the whole file from memory, and manually modifying file.meta (which some methods such as file::touch and metaSave do). Do you have information about which fields are missing, and can you watch the root file's meta property in debug mode?

Another thing. I would be quite surprised if this bug was specific to the / file. The root folder doesn't need to be in memory to access files in the tree. So, yes, your pull request is quite the hack ☺ Which isn't an issue per se, but I'd love to know more about the circumstances of the bug.

jankeromnes commented 11 years ago

Happened again 10h ago, it's in the server logs, and it keeps our uptime from increasing too much at a time. I'll merge @BenjBouv's fix soon if you don't mind @espadrine.

jankeromnes commented 11 years ago

I'll consider this fixed, unless proven otherwise. Let's watch the uptime grow at https://thefiletree.com/profiler.html, and cross-check reboots with server logs.