Jelmerro / Vieb

Vim Inspired Electron Browser - Vim bindings for the web by design
https://vieb.dev
GNU General Public License v3.0
1.32k stars 63 forks source link

[Windows-only] Vieb does not fully quit on Windows because deleting files freezes the entire app #526

Closed Melandel closed 10 months ago

Melandel commented 11 months ago

Checklist

Describe the bug Running Vieb, quitting it, then re-running it from the command line:

To Reproduce

Expected behavior Vieb is open on the wikipedia page

Specs

Additional information Apparently, there are Vieb processes still running after quitting vieb: image

Jelmerro commented 11 months ago

Not quitting correctly with :quitall can be debugged by starting Vieb with --devtools the first time and then doing the :quitall, the window should then stay open and show the error in the devtools. Please share it here so I can know the issue. Is this new in 10.4.0 compared to older versions?

Melandel commented 11 months ago

the window should then stay open and show the error in the devtools. Please share it here so I can know the issue.

Vieb just freezes without showing the devtools.

I tried with the devtools opened before quitting - nothing in the console.

Is this new in 10.4.0 compared to older versions?

I am able to confirm that:

Jelmerro commented 11 months ago

Can you reproduce this with a clean config? Because I cannot reproduce this on Windows 11 at all. And how about a separate datafolder? If there's no error on quit and it keeps running, even when running with --devtools, it either freezes at Chromium level or keeps running one of the while loops in the code indefinitely, of which there are not a lot: https://github.com/search?q=repo%3AJelmerro%2FVieb%20while&type=code And the only one that could possibly run on quit is the rimraf one, which stops after some retries. Let me know if you have any other info, because I cannot reproduce or know the issue if it freezes without errors.

Melandel commented 11 months ago

Can you reproduce this with a clean config? How about a separate datafolder?

I am able to confirm that by removing my VIEB_CONFIG_FILE and VIEB_DATAFOLDER environment parameters, I am able to reproduce the bug on Windows 10 using npm ci; npm start "https://www.bienici.com/recherche/location/paris-75000/2-pieces-et-plus?prix-max=900&chambres-min=1"

I tried adding some console.log inside the function quitall, but they did not show on my terminal.

I tried commenting instructions, and was able to investigate further using that method.

I am able to confirm that commenting out these 2 lines exclusively(here and here) removes the freezing, somehow

Jelmerro commented 11 months ago

So it's basically either Windows that blocks the deletion of some files, or an infinite loop in this script: https://github.com/Jelmerro/Vieb/blob/master/app/rimraf.js Could you figure out where exactly it freezes? Because even after building up some history and files in those directories I managed to quit the browser without error on Windows 10 too.

Edit: console.log from the Vieb process show inside the F8 devtools, not in the console, that's only when logging from app/index.js since that is the main process.

Melandel commented 11 months ago

Thanks for the F8 tip!

Logs

Click me! ```text Vieb\app\rimraf.js:136 [C:\Users\ANON\AppData\Roaming\Vieb\Service Worker] Trying to fetch statistics Vieb\app\rimraf.js:144 [C:\Users\ANON\AppData\Roaming\Vieb\Service Worker] 🚫 Statistics could not be fetched { "errno": -4058, "syscall": "lstat", "code": "ENOENT", "path": "C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\Service Worker" } Vieb\app\rimraf.js:148 [C:\Users\ANON\AppData\Roaming\Vieb\Service Worker] ✅ Abort (ENOENT means "file not found") Vieb\app\rimraf.js:136 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] Trying to fetch statistics Vieb\app\rimraf.js:140 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] ✅ Statistics fetched successfully Vieb\app\rimraf.js:164 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] Trying to remove directory Vieb\app\rimraf.js:44 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] Trying to call fs.rmdirSync Vieb\app\rimraf.js:48 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] ✅ fs.rmdirSync was successful Vieb\app\rimraf.js:168 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\Service Worker] ✅ Directory removed successfully Vieb\app\rimraf.js:77 [C:\Users\ANON\AppData\Roaming\Vieb\blob_storage] Now starting internal fs.rmSync retry strategy (100 retries) Vieb\app\rimraf.js:80 [C:\Users\ANON\AppData\Roaming\Vieb\blob_storage] internal fs.rmSync retry #1 on 100... Vieb\app\rimraf.js:84 [C:\Users\ANON\AppData\Roaming\Vieb\blob_storage] ✅ internal fs.rmSync retry #1 was successful Vieb\app\rimraf.js:77 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\blob_storage] Now starting internal fs.rmSync retry strategy (100 retries) Vieb\app\rimraf.js:80 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\blob_storage] internal fs.rmSync retry #1 on 100... Vieb\app\rimraf.js:84 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\blob_storage] ✅ internal fs.rmSync retry #1 was successful Vieb\app\rimraf.js:136 [C:\Users\ANON\AppData\Roaming\Vieb\databases] Trying to fetch statistics Vieb\app\rimraf.js:144 [C:\Users\ANON\AppData\Roaming\Vieb\databases] 🚫 Statistics could not be fetched { "errno": -4058, "syscall": "lstat", "code": "ENOENT", "path": "C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\databases" } Vieb\app\rimraf.js:148 [C:\Users\ANON\AppData\Roaming\Vieb\databases] ✅ Abort (ENOENT means "file not found") Vieb\app\rimraf.js:136 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] Trying to fetch statistics Vieb\app\rimraf.js:140 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] ✅ Statistics fetched successfully Vieb\app\rimraf.js:164 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] Trying to remove directory Vieb\app\rimraf.js:44 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] Trying to call fs.rmdirSync Vieb\app\rimraf.js:52 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] 🚫 fs.rmdirSync failed { "errno": -4051, "syscall": "rmdir", "code": "ENOTEMPTY", "path": "C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\Partitions\\main\\databases" } Vieb\app\rimraf.js:136 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] Trying to fetch statistics Vieb\app\rimraf.js:140 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] ✅ Statistics fetched successfully Vieb\app\rimraf.js:172 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] Trying to unlink file Vieb\app\rimraf.js:184 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] 🚫 File unlink failed { "errno": -4082, "syscall": "unlink", "code": "EBUSY", "path": "C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\Partitions\\main\\databases\\Databases.db" } Vieb\app\rimraf.js:212 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] Calling rmSync Vieb\app\rimraf.js:44 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] Trying to call fs.rmdirSync Vieb\app\rimraf.js:52 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] 🚫 fs.rmdirSync failed { "errno": -4058, "syscall": "rmdir", "code": "ENOENT", "path": "C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\Partitions\\main\\databases\\Databases.db" } Vieb\app\rimraf.js:56 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases\Databases.db] Abort (ENOENT means "file not found") Vieb\app\rimraf.js:77 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] Now starting internal fs.rmSync retry strategy (100 retries) Vieb\app\rimraf.js:80 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] internal fs.rmSync retry #1 on 100... Vieb\app\rimraf.js:87 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] 🚫 internal fs.rmSync retry #1 failed { "errno": -4082, "syscall": "unlink", "code": "EBUSY", "path": "\\\\?\\C:\\Users\\ANON\\AppData\\Roaming\\Vieb\\Partitions\\main\\databases\\Databases.db" } Vieb\app\rimraf.js:80 [C:\Users\ANON\AppData\Roaming\Vieb\Partitions\main\databases] internal fs.rmSync retry #2 on 100... ```

Analysis

It looks like there is a mix of 3 things:

  1. calling fs.rmSync(p, { "maxRetries": 100, "recursive": true }) on a resource previously found busy ends up looping infinitely, sometimes at the first attempt, sometimes at the second attempt
  2. After failing to unlink a file found busy (EBUSY error), calling fs.rmdirSync wrongfully find it inexistant (ENOENT error despite the file existing in the system explorer) and stops the removal attempt
  3. Why is rimraf trying to delete a directory (fs.rmdirSync) on a file after failing to unlink it?

Additional information

I've been playing the scenario multiple times, and:

Jelmerro commented 11 months ago

This proves to me what the author of Rimraf (see screenshot) and my docs also explain in the rimraf file: Windows file access is fundamentally broken. The reason it is retried a 100 times only on Windows is because on mac or linux if it fails there's a good reason for it, so 1 retry is more than enough, while Windows keeps the file open for too long and thus breaks file removal. If it freezes on any of the fs.* functions, that means that the NodeJS implementation of the filesystem access freezes. This is not something I can solve in Vieb I'm afraid. I am unsure why this is new behavior, as the commit you reference did not change the rimraf file at all, nor change any of the invocations. So either that is not the right commit, or this was not caused by a change in Vieb directly (though possibly it is related to the new NodeJS Chromium and Electron versions that V10 uses compared to 9.7.1). As such, I do not know what I can do to debug or solve this, as working file removal sounds like a fairly basic requirement for having a working OS. I can try to switch back to the more bloated original rimraf module to hope that they have found a fix for this, but I'm skeptical that this is fixable.

image

Melandel commented 11 months ago

I understand.

May something like this help?

Edit: I can confirm that copy/pasting rimraf.js from 9.7.1 prevents the freeze from happening! 😄

Jelmerro commented 11 months ago

Very glad that you found a cause for this in the rimraf file, something must have happened to the Windows handling recently, that still works for mac/linux, as I don't have the ability to test Windows regularly. I'll send you a bunch of different versions of the rimraf file soon to test so we might be able to know what caused this. I'll let you know when I have them ready.

Jelmerro commented 11 months ago

Actually, before checking anything else, I found a potential problem while comparing the diffs. Can you try replacing this at the end of the file of the V10 version:

--- a/app/rimraf.js
+++ b/app/rimraf.js
@@ -138,7 +138,7 @@ const rimrafSync = p => {
             }
             return rmdirSync(p, er)
         }
-        if (isErrnoException(er) && er.code === "EISDIR") {
+        if (!isErrnoException(er) || er.code !== "EISDIR") {
             throw er
         }
         if (er instanceof Error) {

Please let me know if this changes the behavior, if not, I'll still make the different versions of the rimraf file (or just a check which of these commits is the culprit would be great too: https://github.com/Jelmerro/Vieb/commits/master/app/rimraf.js). If the above fixes it, I imagine this would be the problematic commit: https://github.com/Jelmerro/Vieb/commit/ce658e7a506850e50f120b58782a30386b3807d0

Melandel commented 11 months ago

The modification you suggested appears to prevent the freeze from happening 👍

Jelmerro commented 11 months ago

I'll include that change in the next release, thanks for all the help!

Melandel commented 11 months ago

Thanks for your time and energy always 🥇

Jelmerro commented 10 months ago

Fixed in 10.5.0 :tada: