emezeske / lein-cljsbuild

Leiningen plugin to make ClojureScript development easy.
Other
1.1k stars 152 forks source link

"unable to save file" on Windows using lein cljsbuild auto #322

Closed joshuacc closed 9 years ago

joshuacc commented 10 years ago

Not sure if this is the proper place to report my issue, but here it is:

Steps to reproduce

  1. Use Windows 7
  2. Generate a new project with lein new mies test-project.
  3. Run lein cljsbuild auto.
  4. Save a ClojureScript file 2-3 times and watch it compile successfully.
  5. Save again

    Result

cljsbuild-error

Further notes

ivan commented 10 years ago

I'm guessing lein-cljsbuild or the ClojureScript compiler is keeping file handles open to .cljs files, which on Windows prevents programs from being able to delete or rename over the files. It should instead always close the handles as soon as possible, or use the new Java 7 APIs that can open files with the FILE_SHARE_DELETE share mode.

As an (untested) workaround, you can change "atomic_save": true, to "atomic_save": false, in your Sublime Text 3 preferences.

ivan commented 10 years ago

And just for IntelliJ IDEA users on Windows who might have the same problem (it shows a "Cannot Save Files" dialog): you can disable IDEA's equivalent of "atomic save" in Settings -> General -> [ ] (uncheck) Use "safe write"

joshuacc commented 10 years ago

@ivan Thanks for the suggestion. Unfortunately changing my Sublime settings doesn't work. But I do get a different error now:

cljsbuild-error2

shinych commented 10 years ago

I confirm this issue (Win7, Sublime3).

Moreover, I experience another issue which possibly has similar root:

In our scenario we watch the output directory (:output-dir) for compiled artifacts with http://docs.oracle.com/javase/7/docs/api/java/nio/file/WatchService.html. Under Windows, the watcher event is fired only after the first automatic compilation.

I have used the ProcessExplorer to observe the issue: cljsbuild holds the handles to all of the compiled resources for some time (approx 80 seconds) and only then releases them.

shinych commented 10 years ago

This comment in cljsbuild.util seems to be relevant: https://github.com/emezeske/lein-cljsbuild/blob/62a08ca56099f0f3b49da9f560d61dd774b4dc42/support/src/cljsbuild/util.clj#L73

Gregg8 commented 10 years ago

Confirmed with Win 8.1 and Sublime 3. Setting atomic_save to false didn't help either.

FYI, Sublime 2 does not have this issue, so must save files in a different way (perhaps it just immediately overwrites).

Would be grateful for a fix or any further suggestions.

joerupen commented 10 years ago

Maybe this issue is not related to cljsbuild, but to clojurescript itself.

cljsbuild relies on the cljs.closure/build function to do the compilation. Commenting out just that line of code makes the file-lock disappear. In the code below I just replace the call to the clojurescript compiler with a simple file output. I don't infer that the problem must be caused from clojurescript itself, but it think it's likely after seeing this behavior. What do you think?

 (defn- compile-cljs [cljs-paths compiler-options notify-command incremental? assert? watching?]
  .... replace
      (build (SourcePaths. cljs-paths) compiler-options))
  .... with
       (spit output-file "var foo = 1;")

NOTE: I just built lein-cljsbuild 1.0.3 with the latest clojurescript dependency 0.0-2311 and the problem still exists.

joerupen commented 10 years ago

Is anyone working on this issue? I posted an issue on ClojureScript (http://dev.clojure.org/jira/browse/CLJS-841) but it looks like it's not related to ClojureScript. If someone could give me hint, I might go through the source and see if I can fix it.

cemerick commented 10 years ago

@shinych AFAICT, process-start is only used in conjunction with running external processes (i.e. for the deprecated cljsbuild REPLs), and the unclosed writers are for stdout and stdin, not any ClojureScript source files.

It's been some time since I've had to deal with windows file locking issues. However, my recollection is that a file is only locked when it's actually being read; the only thing being done when auto is used is that modification times are collected and compared (every 100ms). Maybe that also implicates a lock, but I'd hope not. If it does, then the only workaround might be to use JDK7 watchers (i.e. #229) when available.

I wonder just how reliable and consistent this problem is? Not a lot has changed in cljsbuild for some time, and I presume there's a large population of windows users that have been using auto over the months/years...?

shinych commented 10 years ago

@cemerick I guess it has to do with the way we use the Watchers, since this issue is constantly reproducible with our code, and nobody else seems to have reported it. Let me check back after I'm back from vacations )

joerupen commented 10 years ago

If the problem is in the watchers, how comes I can overwrite the source files in sublime as soon as I comment out the invocation to cljs.closure/build ?

cemerick commented 10 years ago

@joerupen I don't have a solid theory. However, cljs.closure always uses slurp to obtain sources, and cljs.analyzer's use of readers looks sane and safe, so I would doubt that the ClojureScript compiler is at fault.

joerupen commented 10 years ago

@cemerick I agree that slurp does not keep open file handles around, but what I can reproduce is that by simply commenting out the call to the build function the locks disappear (or they are much less frequent, as I didn't notice them). Maybe the issue is due to a combination of lein-cljsbuild and cljs.closure/build. In fact, when I just run the cljs.closure/build function from the repl, no locks occur, which could prove your theory. But if the cljs.closure/build function was not the problem, then I would expect that the file-watching mechanism is the reason and I should be able to reproduce it without calling the build function (which I can't). The plugin polls the watched directories every 100ms and queries the modified dates of the source files. That alone could cause the problem. I tried to increase the interval to 5 seconds and that helps a little, locks become much rarer (in order of magnitude). My current workaround is to inject a Thread/sleep of 5 sec right after the call to cljs.closure/build, then keep polling for file changes every 100ms. Doesn't this sound to you like a finalizer waiting to be called by the garbage collector? When the polling continues right after the build, the lock is much more likely to occur. When I give it some time to relax, the locks become rare. I wouldn't be surpised if that was due to how the JVM works though. I am using JDK 7. If that's the case then only a hack could possibly fix it and that's not probably what you're after.

joerupen commented 10 years ago

The following code snippet helps as a workaround for sublime: when clojure.cljs/build reads from a temp folder, there is no conflict whatsoever. Just replace https://github.com/emezeske/lein-cljsbuild/blob/master/support/src/cljsbuild/compiler.clj#L58 with (build-sublime cljs-paths compiler-options).

(defn build-sublime [paths opts]
  (let [dir (fs/temp-dir)
        temp-paths   (vec (map #(-> % (fs/copy-dir dir) fs/absolute-path) paths))
        ret (build (SourcePaths. temp-paths) opts)]
          (fs/delete-dir dir)
          ret))

What is interesting is that I delete the temp folder. If cljs.closure/build had a lock on the source files, the delete operation would fail but it doesnt. With this patch, both ClojureScript and lein-cljsbuild seem to be working, so none of them has a bug? This issue only occurs when both operate on the same folder? That's a heisenbug I guess.

cemerick commented 9 years ago

Seems like an orphan issue. I'm happy to take a patch that improves something on Windows, but what's going on here sounds ambiguous at best. Closing for now; feel free to reopen if anything concrete can be determined / fixed.