denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.6k stars 5.25k forks source link

Deno exceeds OS open files limit (os error 24) when NodeJS does not #17757

Open bershanskiy opened 1 year ago

bershanskiy commented 1 year ago

Some tools, notably project bundlers, work by observing a large number of files (typically the entire project folder). NodeJS can handle the OS limit on number of open files while Deno does not.

Please note:

Repro steps

demo.mjs

// Adjust this parameter if needed, the critical number will depend on your system state
const watcherCount = 100;

import { watch } from 'node:fs';

async function main() {
  for (let i = 0; i < watcherCount; i++) {
    console.log(`Creating watcher ${i}`);
    watch('./a.txt', (d) => console.log(`'${d}' received by watcher ${i}`));
  }
  return new Promise(() => {});
}

await main();

Commands

  1. Create a file to watch: touch a.txt
  2. Run node with node demo.mjs or deno with deno run --allow-read=a.txt demo.mjs or both
  3. Change file being watched: echo 1 > a.txt
  4. Observe that node prints out a bunch of change logs while deno fails

Error

error: Uncaught Error: Too many open files (os error 24)
    iterator = Deno.watchFs(watchPath, {
                    ^
    at new FsWatcher (internal:runtime/js/40_fs_events.js:17:21)
    at Object.watchFs (internal:runtime/js/40_fs_events.js:60:10)
    at https://deno.land/std@0.177.0/node/_fs/_fs_watch.ts:119:21
    at Object.action (internal:deno_web/02_timers.js:146:11)
    at handleTimerMacrotask (internal:deno_web/02_timers.js:63:10)
bershanskiy commented 1 year ago

I believe there are two errors here:

  1. runtime/ops/fs_events.rs creates a new unique watcher for every call to Deno.watchFs (that is op_fs_events_open). deno runtime could create a single watcher on first call to op_fs_events_open and then reuse it for all subsequent calls.
  2. Since Watcher::new() may throw if OS limit is reached, Deno.watchFs probably should capture the exception and relay it to the caller instead of crashing
aapoalas commented 1 year ago
  1. Since Watcher::new() may throw if OS limit is reached, Deno.watchFs probably should capture the exception and relay it to the caller instead of crashing

The error from Watcher::new() is being thrown to the user as an error. Your reproduction code just lacks a try-catch so the error becomes an uncaught error which closes the program.

The problem of reaching the open files limit is still valid, of course.

bershanskiy commented 1 year ago

The error from Watcher::new() is being thrown to the user as an error. Your reproduction code just lacks a try-catch so the error becomes an uncaught error which closes the program.

I probably did not formulate this clearly enough. The above example does not have a try-catch, but when I add try-catch I still can not catch the error. For minimal repro case please consider this:

  1. Start deno in repl without permissions at all (to make sure watch fails because of permissions): deno repl
  2. Import watch: import { watch } from 'node:fs';
  3. Call watch() in a try-catch and deny permissions to ensure watch() fails: try { watch('denied', () => {}) } catch (e) { console.log('not printed') }
  4. Observe exception and program termination

Here is a repl run:

$ deno repl
Deno 1.30.3+0aeb8bc
exit using ctrl+d, ctrl+c, or close()
> import { watch } from 'node:fs';
undefined
> try { watch('denied', () => {}) } catch (e) { console.log('not printed') }
❌ Denied read access to "denied".
error: {"code":-32000,"message":"Execution was terminated"}

I'm not entirely sure, but this might be caused by setTimeout() in node:fs.watch(): https://github.com/denoland/deno/blob/main/ext/node/polyfills/_fs/_fs_watch.ts#L118

If I remove this setTimeout() and run the same test I get the expected behavior (error can be caught):

$ target/debug/deno repl
Deno 1.30.3
exit using ctrl+d, ctrl+c, or close()
> import { watch } from 'node:fs';
undefined
> try { watch('denied', () => {}) } catch (e) { console.log('not printed') }
❌ Denied read access to "denied".
not printed
undefined
bershanskiy commented 1 year ago

I'm still observing both issues on Deno 1.32.3.

mliudev commented 1 year ago

I think I'm also experiencing a related issue. I have a simple deno script that runs npm:tsc for typescript compilation and another deno script which uses npm:chokidar in conjunction with npm:prettier to watch files. The scripts are as follows:

tsc.sh:

exec deno run \
    --allow-env \
    --allow-read \
    --allow-write \
    'npm:typescript@5.1.6/tsc' \
    --watch \
    $@

format.sh:

#!/bin/bash

TOP=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

# verify required binaries are available
for BINARY in deno; do
    if ! type "$BINARY" > /dev/null 2>&1; then
        echo "Required binary not found: $BINARY"
        exit 1
    fi
done

# start prettier formatter
deno run \
    --allow-sys \
    --allow-read \
    --allow-env \
    --allow-run \
    "npm:chokidar-cli" \
        "**/*.ts" \
        "**/*.html" \
        "**/*.css" \
        -c "deno run \
            --allow-read \
            --allow-write \
            --allow-env \
            'npm:prettier' \
            "${TOP}" --check --write"

I get the following error:

[4:25:13 PM] Starting compilation in watch mode...

[4:25:13 PM] Found 0 errors. Watching for file changes.

error: Uncaught Error: Too many open files (os error 24)
    at new FsWatcher (ext:runtime/40_fs_events.js:17:21)
    at Object.watchFs (ext:runtime/40_fs_events.js:60:10)
    at ext:deno_node/_fs/_fs_watch.ts:58:21
    at Object.action (ext:deno_web/02_timers.js:153:11)
    at handleTimerMacrotask (ext:deno_web/02_timers.js:67:10)
    at eventLoopTick (ext:core/01_core.js:189:21)
rlidwka commented 9 months ago

I have the same issue when watching files on vite+typescript project.

Solved by increasing inotify limit on my system (it's only 128 by default):

echo 10240 | sudo tee /proc/sys/fs/inotify/max_user_instances

I'm curious why it works out of the box on node.js though.

jamesdiacono commented 6 months ago

The call to Deno.watchFs is made in a setTimeout callback with no try/catch, so the exception can not be caught and the process crashes: https://github.com/denoland/deno/blob/61f1b8e8dc20846093a8b24a8f511a09bbf09919/ext/node/polyfills/_fs/_fs_watch.ts#L118-L124

Apparently the reason for the setTimeout is to avoid a race condition in the test. Perhaps the test could be improved to avoid the need for a kludge?

littledivy commented 5 months ago

I'm curious why it works out of the box on node.js though.

Deno uses the inotify API (via notify-rs) under the hood on Linux whereas IIUC Node.js uses a (much slower) polling-based watching mechanism. Most software that uses inotify runs into this problem and increasing the OS limit is probably the best solution to this. I'll bring this up internally but I would much prefer to stick with OS APIs and ask users to increase the limit.

Btw, The error thrown should be catchable so that's a bug for sure.

ninjadev64 commented 2 months ago

I'm getting this while trying to run one of my SvelteKit projects with Vite. Curiously, it doesn't happen with the other projects I migrated to Deno...

bzm3r commented 1 month ago

I'm curious why it works out of the box on node.js though.

Deno uses the inotify API (via notify-rs) under the hood on Linux whereas IIUC Node.js uses a (much slower) polling-based watching mechanism. Most software that uses inotify runs into this problem and increasing the OS limit is probably the best solution to this. I'll bring this up internally but I would much prefer to stick with OS APIs and ask users to increase the limit.

Btw, The error thrown should be catchable so that's a bug for sure.

@littledivy How large should the inotify limit be? Are there any issues one might run into if it's "too large"?

I tried running the following command based on what I cobbled together from the internet:

echo fs.inotify.max_user_watches=524288 | sudo tee -a /etc/sysctl.conf && sudo sysctl -p

but I still get the same error.

EDIT: I wonder if it might be because due to an error on my side (at least in my case), because I am in the midst of upgrading a Svelte 3 project to a Svelte 5 project? I'll keep troubleshooting and update here when I do learn more.

phoenisx commented 1 month ago

I think there's some issue with chokidar and deno.

I turned off useFsEvents option for my svelte project, and am using usePolling to make it work for now.