likely fixes issue with occasional file rename race on linux

[zapu]

As discussed previously (in October... wow), I can still get it to fail on Linux.

I think while the lock protects individual reads, there is still a chance for unfavorable ordering of callbacks, so our compilation of empty template can come up last and overwrite the viewCache.

I have a script that reproduces it sometimes: https://gist.github.com/zapu/173bd8dc07ce8c2210fefcaef5f2d4a4

It can be put in toffee directory ran ran from there. It tries to write same content to the same file two times, then waits 1 second, and then runs the template. If it gets empty results back, it means that empty file was compiled and set. To confirm, it also reads the file back using fs.readFileSync and prints current contents.

I'm on Ubuntu:

No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 19.04
Release:    19.04
Codename:   disco

Linux wakaba 5.0.0-37-generic #40-Ubuntu SMP Thu Nov 14 00:14:01 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

[zapu]

I'm trying with the following change now - only releasing lock when view is done compiling. This should provide complete serialization of subsequent reads and compilations.

diff --git a/src/engine.coffee b/src/engine.coffee
index feb1b74..9db79e1 100644
--- a/src/engine.coffee
+++ b/src/engine.coffee
@@ -232,6 +232,7 @@ class engine
     @fileLockTable.acquire2 {name: filename}, (lock) =>
       fs.readFile filename, 'utf8', (err, txt) =>
         @_log "#{Date.now()} - #{filename} changed to #{txt?.length}bytes. #{txt?.replace?(/\n/g , '')[...80]}"
+        waiting_for_view = false
         if err or (txt isnt @viewCache[filename].txt)
           if err
             @fsErrorCache[filename] = Date.now()
@@ -245,11 +246,15 @@ class engine
               @_log "#{filename} updated and ready"
               @viewCache[filename] = v
               @pool.release(ctx)
+              @_log "#{filename} lock releasing (view_options.cb)"
+              lock.release()
             if err
               view_options.fsError = true
+            waiting_for_view = true # do not release lock instantly
             v = new view txt, view_options
-        @_log "#{filename} lock releasing"
-        lock.release()
+        if not waiting_for_view
+          @_log "#{filename} lock releasing (not waiting for view)"
+          lock.release()

   _generateViewOptions: (filename) ->
     return {

[zapu]

I just tried it on MacOS and the behavior is completely different. It doesn't even seem to get two fs notifications when stress script writes file twice.

For the record, the issue here is likely not a rename race. Looks like writeFile in node (which is also used to save files in VSCode) will first truncate the file and then write the contents, as two separate writes. Not sure if there's anything else one can do, this behavior seems to be consistent with what e.g. echo "" > x.toffee would do.

Related discussions: https://github.com/nodejs/node/issues/6112 https://github.com/microsoft/vscode/issues/9419

[zapu]

@malgorithms another look and lets release it? thanks

[malgorithms]

LGTM