macOS: Crash when using `fs.watch` for network drives

JCervero commented 4 years ago

On MacOS 10.15 when a file is open from an SMB share if connection t othe SMB share is lost the entire editor crashes.

To repro: Open finder and Connect to Server open a file from the mapped SMB drive in VS Code disconnect the network return to VS Code VS Code crashes

Expected behavior would be 'file no longer exists' or something similar

bpasero commented 4 years ago

Can you please follow the steps in https://github.com/Microsoft/vscode/wiki/Native-Crash-Issues to get at more details around the crash and attach the result here? Thanks!

JCervero commented 4 years ago

a7dc75ff-3eae-44fb-96eb-230aee12be2b.dmp.zip

Do I need to re-submit this issue or will it automatically re-open?

deepak1556 commented 4 years ago

Symbolicated trace

Operating system: Mac OS X
                  10.15.6 19G2021
CPU: amd64
     family 6 model 70 stepping 1
     8 CPUs

GPU: UNKNOWN

Crash reason:  0x00000000 / 0x00000000
Crash address: 0x7fff6949333a
Process uptime: 920 seconds

Thread 0 (crashed)
 0  libsystem_kernel.dylib!__pthread_kill + 0xa
    rax = 0x0000000000000000   rdx = 0x0000000000000000
    rcx = 0x00007ffee0c40278   rbx = 0x00000001141fedc0
    rsi = 0x0000000000000006   rdi = 0x0000000000000307
    rbp = 0x00007ffee0c402a0   rsp = 0x00007ffee0c40278
     r8 = 0x0000000000000000    r9 = 0x0000000000000000
    r10 = 0x00000001141fedc0   r11 = 0x0000000000000246
    r12 = 0x0000000000000307   r13 = 0x0000000000000000
    r14 = 0x0000000000000006   r15 = 0x0000000000000016
    rip = 0x00007fff6949333a
    Found by: given as instruction pointer in context
 1  libsystem_c.dylib!abort + 0x78
    rbp = 0x00007ffee0c402e0   rsp = 0x00007ffee0c402b0
    rip = 0x00007fff6941a808
    Found by: previous frame's frame pointer
 2  Electron Framework!uv__fs_event [kqueue.c : 442 + 0x5]
    rbp = 0x00007ffee0c40750   rsp = 0x00007ffee0c402f0
    rip = 0x0000000119e74f01
    Found by: previous frame's frame pointer
 3  Electron Framework!uv__io_poll [kqueue.c : 279 + 0x8]
    rbp = 0x00007ffee0c48810   rsp = 0x00007ffee0c40760
    rip = 0x0000000119e749bb
    Found by: previous frame's frame pointer
 4  Electron Framework!uv_run [core.c : 375 + 0x8]
    rbp = 0x00007ffee0c48880   rsp = 0x00007ffee0c48820
    rip = 0x0000000119e63e71
    Found by: previous frame's frame pointer
 5  Electron Framework!electron::NodeBindings::UvRunOnce() [node_bindings.cc : 434 + 0xa]
    rbp = 0x00007ffee0c48910   rsp = 0x00007ffee0c48890
    rip = 0x000000011439e76e
    Found by: previous frame's frame pointer
 6  Electron Framework!base::TaskAnnotator::RunTask(char const*, base::PendingTask*) [callback.h : 98 + 0x3]
    rbp = 0x00007ffee0c489c0   rsp = 0x00007ffee0c48920
    rip = 0x000000011676cb6f
    Found by: previous frame's frame pointer
 7  Electron Framework!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*, bool*) [thread_controller_with_message_pump_impl.cc : 324 + 0xf]
    rbp = 0x00007ffee0c48a90   rsp = 0x00007ffee0c489d0
    rip = 0x000000011677cd4a
    Found by: previous frame's frame pointer
 8  Electron Framework!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() [thread_controller_with_message_pump_impl.cc : 248 + 0xb]
    rbp = 0x00007ffee0c48ae0   rsp = 0x00007ffee0c48aa0
    rip = 0x000000011677cad9
    Found by: previous frame's frame pointer
 9  Electron Framework!invocation function for block in base::MessagePumpCFRunLoopBase::RunWorkSource(void*) [message_pump_mac.mm : 499 + 0x6]
    rbp = 0x00007ffee0c48b00   rsp = 0x00007ffee0c48af0
    rip = 0x00000001167cd561
    Found by: previous frame's frame pointer
10  Electron Framework!base::mac::CallWithEHFrame(void () block_pointer) + 0xa
    rbp = 0x00007ffee0c48b10   rsp = 0x00007ffee0c48b10
    rip = 0x00000001167c8cfa
    Found by: previous frame's frame pointer
11  Electron Framework!base::MessagePumpCFRunLoopBase::RunWorkSource(void*) [message_pump_mac.mm : 475 + 0x5]
    rbp = 0x00007ffee0c48b50   rsp = 0x00007ffee0c48b20
    rip = 0x00000001167cce2f
    Found by: previous frame's frame pointer
12  CoreFoundation!__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 0x11
    rbp = 0x00007ffee0c48b60   rsp = 0x00007ffee0c48b60
    rip = 0x00007fff2f2b4d52
    Found by: previous frame's frame pointer
13  CoreFoundation!__CFRunLoopDoSource0 + 0x67
    rbp = 0x00007ffee0c48b90   rsp = 0x00007ffee0c48b70
    rip = 0x00007fff2f2b4cf1
    Found by: previous frame's frame pointer
14  CoreFoundation!__CFRunLoopDoSources0 + 0xd1
    rbp = 0x00007ffee0c48c00   rsp = 0x00007ffee0c48ba0
    rip = 0x00007fff2f2b4b0b
    Found by: previous frame's frame pointer
15  CoreFoundation!__CFRunLoopRun + 0x39f
    rbp = 0x00007ffee0c49910   rsp = 0x00007ffee0c48c10
    rip = 0x00007fff2f2b383a
    Found by: previous frame's frame pointer
16  CoreFoundation!CFRunLoopRunSpecific + 0x1ce
    rbp = 0x00007ffee0c499a0   rsp = 0x00007ffee0c49920
    rip = 0x00007fff2f2b2e3e
    Found by: previous frame's frame pointer
17  Foundation!-[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 0xd4
    rbp = 0x00007ffee0c499e0   rsp = 0x00007ffee0c499b0
    rip = 0x00007fff3194e1c8
    Found by: previous frame's frame pointer
18  Electron Framework!base::MessagePumpNSRunLoop::DoRun(base::MessagePump::Delegate*) [message_pump_mac.mm : 745 + 0x10]
    rbp = 0x00007ffee0c49a20   rsp = 0x00007ffee0c499f0
    rip = 0x00000001167cda81
    Found by: previous frame's frame pointer
19  Electron Framework!base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) [message_pump_mac.mm : 190 + 0xc]
    rbp = 0x00007ffee0c49a60   rsp = 0x00007ffee0c49a30
    rip = 0x00000001167cc7e2
    Found by: previous frame's frame pointer
20  Electron Framework!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) [thread_controller_with_message_pump_impl.cc : 429 + 0x6]
    rbp = 0x00007ffee0c49a90   rsp = 0x00007ffee0c49a70
    rip = 0x000000011677d2d3
    Found by: previous frame's frame pointer
21  Electron Framework!base::RunLoop::Run() [run_loop.cc : 124 + 0x13]
    rbp = 0x00007ffee0c49b20   rsp = 0x00007ffee0c49aa0
    rip = 0x00000001167550b3
    Found by: previous frame's frame pointer
22  Electron Framework!content::RendererMain(content::MainFunctionParams const&) [renderer_main.cc : 226 + 0x5]
    rbp = 0x00007ffee0c49be0   rsp = 0x00007ffee0c49b30
    rip = 0x0000000119bb9d54
    Found by: previous frame's frame pointer
23  Electron Framework!content::ContentMainRunnerImpl::Run(bool) [content_main_runner_impl.cc : 882 + 0x5]
    rbp = 0x00007ffee0c49c50   rsp = 0x00007ffee0c49bf0
    rip = 0x0000000114ebbb49
    Found by: previous frame's frame pointer
24  Electron Framework!service_manager::Main(service_manager::MainParams const&) [main.cc : 454 + 0x9]
    rbp = 0x00007ffee0c49fa0   rsp = 0x00007ffee0c49c60
    rip = 0x0000000117f0be08
    Found by: previous frame's frame pointer
25  Electron Framework!content::ContentMain(content::ContentMainParams const&) [content_main.cc : 19 + 0x8]
    rbp = 0x00007ffee0c4a030   rsp = 0x00007ffee0c49fb0
    rip = 0x0000000114ebb224
    Found by: previous frame's frame pointer
26  Electron Framework!ElectronMain [electron_library_main.mm : 31 + 0x5]
    rbp = 0x00007ffee0c4a0c0   rsp = 0x00007ffee0c4a040
    rip = 0x000000011423bbd4
    Found by: previous frame's frame pointer
27  Code Helper (Renderer)!main [electron_main.cc : 287 + 0xb]
    rbp = 0x00007ffee0c4a190   rsp = 0x00007ffee0c4a0d0
    rip = 0x000000010efb68a5
    Found by: previous frame's frame pointer
28  libdyld.dylib!start + 0x1
    rbp = 0x00007ffee0c4a1a0   rsp = 0x00007ffee0c4a1a0
    rip = 0x00007fff6934bcc9
    Found by: previous frame's frame pointer

deepak1556 commented 4 years ago

Abort in libuv from here https://github.com/libuv/libuv/blob/v1.x/src/unix/kqueue.c#L482

deepak1556 commented 4 years ago

We should be able to create a minimal example with electron or node.js , @bpasero where is the code that handles opening files from SMB drive, need some idea to create a repro. Thanks!

bpasero commented 4 years ago

@deepak1556 great findings, I think we had this issue for a long time without understanding it fully. Most files are being opened here:

https://github.com/microsoft/vscode/blob/e8760a5d6c0e09ab692e5b3e414eca018745490f/src/vs/platform/files/node/diskFileSystemProvider.ts#L209-L209

And then we use normal fs.read primitives from node to read and fs.close.

deepak1556 commented 3 years ago

I was able to narrow down the source of issue, the crash only happens when using fs.watch to monitor files from SMB.

fs.watch(filePath); // will crash when SMB disconnects

According https://github.com/nodejs/node/issues/18073#issuecomment-356538888 and https://nodejs.org/dist/latest-v14.x/docs/api/fs.html#fs_availability it is not advised to use fs.watch for network drive files.

Tracing our code, looks like we use it from NodeJSWatcherService when relying on non-recursive watching mode https://github.com/microsoft/vscode/blob/21dc8580c949d33e6aff1152e69d9dd8251c1c1b/src/vs/platform/files/node/diskFileSystemProvider.ts#L539-L545

There are two solutions:

1) Resort to recursive file watchers for network drive files 2) Use fs.watchFile in non-recursive watcher for network drive files

bpasero commented 3 years ago

@deepak1556 wow great findings 👍 , now do you know any way to figure out whether a file path is a SMB share or not? Is it as easy as checking for \\ in the path beginning? And is this only on POSIX or Windows too (I think windows has this notation reserved for UNC paths).

deepak1556 commented 3 years ago

fs.watch can be unreliable for network filesystems on both posix and windows but currently we are only observing the crash on macOS.

Is it as easy as checking for \ in the path beginning?

I am not aware of a specific way to figure this, the problem here is that we also want to identify the mounted paths. Maybe a simpler check would be to see if the path starts with /Volumes for default mount point cases ?

Also what about providing a setting to opt-in the recursive file watchers for affected users ?

bpasero commented 3 years ago

I can see to add some heuristics but would stay away from having a setting imho.

bpasero commented 3 years ago

I have pushed a somewhat weak workaround to ignore any path contained in /Volumnes when using fs.watch:

https://github.com/microsoft/vscode/blob/53c3a2297bf33474d79e584c292d7068ece02800/src/vs/base/node/watcher.ts#L26-L34

But I feel this is just a bandaid, because:

imho a native crash should never ever occur when using an official API no matter what, at the very minimum the upstream component should log an error and refuse to watch but never just crash
looking at all the other issues that were marked as duplicate of this one, I see users having mapped network drives on other paths than /Volumnes possibly using symlinks, so I doubt we would catch all cases here

Back to you Deepak, I think this needs a different upstream fix if possible and better understanding of the origin of the assertion.

tcpekin commented 3 years ago

I opened (the now closed) issue #130978 due to this bug. As @bpasero mentions in his second bullet point, I do not mount my drives to /Volumes/ but rather ~/mnt/, and so VSCode still crashes. Thank you very much for your work on this issue, it seems like there is an understanding as to why this happens, but if there is any way I can help please let me know.

bpasero commented 3 years ago

Thanks for letting us know, I think my fix is not doing anything useful then.

marko-asplund commented 3 years ago

I'm also experiencing crashes on macOS with Google Drive mounted files #130568.

DavidGoldman commented 3 years ago

Abort in libuv from here https://github.com/libuv/libuv/blob/v1.x/src/unix/kqueue.c#L482

I'm guessing (haven't confirmed it yet) that the issue is that an event is fired (e.g. maybe a rename or an ERROR event) when the remote filesystem is unmounting and this line causes the crash since kevent fails. I wonder if EBADF or ENOENT should be ignored like here, or perhaps the error/badfd can be detected earlier/there and invalidated.

daeh commented 2 years ago

I'm having this issue too

(When I run code --crash-reporter-directory, it only generates settings.dat and empty directories, no .dmp file.)

Version: 1.63.2 Commit: 899d46d82c4c95423fb7e10e68eba52050e30ba3 Date: 2021-12-15T09:38:17.605Z Electron: 13.5.2 Chromium: 91.0.4472.164 Node.js: 14.16.0 V8: 9.1.269.39-electron.0 OS: Darwin arm64 20.6.0

Thanks for working on it

JuliusNmn commented 9 months ago

I'm trying to use VSCode with Unreal Engine installed on an external SSD and your patch breaks my setup. I'm getting a bunch of these:

2024-02-06 00:02:31.459 [error] [File Watcher (node.js)] Refusing to watch /Volumes/T7 1/EpicGames/UE_5.3/Engine/Source/Programs/Shared/EpicGames.OIDC/obj/Debug/net6.0 for changes using fs.watch() for possibly being a network share where watching is unreliable and unstable.
2024-02-06 00:02:31.459 [error] [File Watcher (node.js)] Refusing to watch /Volumes/T7 1/EpicGames/UE_5.3/Engine/Source/Programs/Shared/EpicGames.AspNet/obj/Debug/net6.0 for changes using fs.watch() for possibly being a network share where watching is unreliable and unstable.
2024-02-06 00:02:31.460 [error] [File Watcher (node.js)] Refusing to watch /Volumes/T7 1/EpicGames/UE_5.3/Engine/Source/Programs/Shared/EpicGames.Perforce.Managed/obj/Debug/net6.0 for changes using fs.watch() for possibly being a network share where watching is unreliable and unstable.

Will revert to an older version...

ipcjs commented 7 months ago

Determining whether it is a network disk by path prefix is too stupid. 😑 Because external disks are also mounted to this path by default.

https://github.com/microsoft/vscode/blob/53c3a2297bf33474d79e584c292d7068ece02800/src/vs/base/node/watcher.ts#L26-L34

microsoft / vscode

macOS: Crash when using `fs.watch` for network drives #106879