baudehlo / node-fs-ext

Extras missing from node's fs module
MIT License
101 stars 45 forks source link

Hangs forever on multiple simultaneous calls #79

Open paragi opened 5 years ago

paragi commented 5 years ago

Thank you for at very nice extension! I have a critical issue I need to have solved:

The following example code, updates and returns a counter, stored in a file. If I call the function more than a handful of times, it hangs forever, apparently in the fs.read function. Would you please look into this?

var fs = require('fs-ext');
const counter = (callback) => {
  fs.open('counter.txt',"r+", (err,fd) => {
    if(err) throw err;
    fs.flock(fd, 'ex', (err) => {
      if(err) throw err;
      let buffer = Buffer.alloc(20);
      fs.read(fd, buffer,0, buffer.length, 0 , (err, readCount, buffer) => {
        if(err) throw err;
        var count = 1 + (parseInt(buffer.toString('utf8',0,readCount)) || 0);
        fs.write(fd,count + " ",0,(err) => {
          if (err) throw err;
          fs.close(fd, () =>{
            if(typeof callback == 'function')  callback(count);
          });
        });
      });
    });
  });
};

fs.writeFile('counter.txt',' '.repeat(20),(err) => {
  for(let i=0; i<100; i++)
    counter(console.log);
});

A appreciate the effort and would like to suggest some other useful add-ons. My understanding of the node framework and windows is minimal. But if you are interested, I might find time to write some useful extension in C++. Would you be interested in integrating that?

Best regrads Simon Rigèt

baudehlo commented 5 years ago

Have you tried adding in an unlock call too? fs.flock(fd, 'un', cb)

On Mon, Nov 5, 2018 at 6:14 AM Simon Rigét notifications@github.com wrote:

Thank you for at very nice extension! I have a critical issue I need to have solved:

The following example code, updates and returns a counter, stored in a file. If I call the function more than a handful of times, it hangs forever, apparently in the fs.read function. Would you please look into this?

var fs = require('fs-ext'); const counter = (callback) => { fs.open('counter.txt',"r+", (err,fd) => { if(err) throw err; fs.flock(fd, 'ex', (err) => { if(err) throw err; let buffer = Buffer.alloc(20); fs.read(fd, buffer,0, buffer.length, 0 , (err, readCount, buffer) => { if(err) throw err; var count = 1 + (parseInt(buffer.toString('utf8',0,readCount)) || 0); fs.write(fd,count + " ",0,(err) => { if (err) throw err; fs.close(fd, () =>{ if(typeof callback == 'function') callback(count); }); }); }); }); }); };

fs.writeFile('counter.txt',' '.repeat(20),(err) => { for(let i=0; i<5; i++) counter(console.log); });

A appreciate the effort and would like to suggest some other useful add-ons. My understanding of the node framework is minimal. But if you are interested, I might find time to write some useful extension in C++. Would you be interested in integrating that? Also I'm

Best regrads Simon Rigèt

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/baudehlo/node-fs-ext/issues/79, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY6ysn8ksDxgDE1rawDmHhS_rhk8Rks5usB2igaJpZM4YOO9C .

paragi commented 5 years ago

Yes. No difference (as it should be according to man page ) I'v tested on Debian ubuntu 18.04. Node 8 - 11 (There are compiler issues with node 11) Are you able to recreate?

baudehlo commented 5 years ago

Sadly I don't have time right now to work on it. Maybe someone else will have a suggestion.

On Mon, Nov 5, 2018 at 11:50 AM Simon Rigét notifications@github.com wrote:

Yes. No difference (as it should be acording to man page ) I'v tested on Debian ubuntu 18.04 Are you able to recreate?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/baudehlo/node-fs-ext/issues/79#issuecomment-435947780, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY5aJWU4UEJcPLnuTzTn5V-CR8OIPks5usGxZgaJpZM4YOO9C .

davedoesdev commented 5 years ago

It's because all libuv's threads are occupied. Either increase the limit using UV_THREADPOOL_SIZE=100 (for example), or lock with exnb and handle the EAGAIN and EWOULDBLOCK errors.

mo22 commented 2 years ago

This is a very difficult issue to run into as it is quite hard to debug.

There is a different library at https://github.com/joyent/node-lockfd that handles the locking call in an extra thread and not via uv's threadpool.

If this project is still alive I could prepare a PR to update this module accordingly (maybe trying nonblocking first for performance and on EAGAIN spawn a thread for the blocking call)

paulocoghi commented 2 years ago

@mo22

The README of https://github.com/joyent/node-lockfd says the library "has been crafted specifically to work on SmartOS".

Do you know if it works on Linux too? If it doesn't work, your PR will be even more important!

mo22 commented 2 years ago

@paulocoghi Both linux and mac seem to work fine, windows seems not to be supported.

It looks as if this project is not very active (many open PRs) and I would like to make sure that this would get merged, maybe also adding typing information.

paulocoghi commented 2 years ago

Thanks for the reply.

It looks as if this project is not very active (many open PRs)

Unfortunately you are right and using the Joyent module seems to be the best option for now.

baudehlo commented 2 years ago

Honestly I'd be happy to pass on maintainership - I don't use this module in any active projects so it's impossible for me to validate PRs.

On Mon, Oct 11, 2021 at 10:10 AM Paulo Coghi @.***> wrote:

Thanks for the reply.

It looks as if this project is not very active (many open PRs)

Unfortunately you are right and using the Joyent module seems to be the best option for now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/baudehlo/node-fs-ext/issues/79#issuecomment-940067539, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFBWY3SEJRAFSBXYBJUAQDUGLV5HANCNFSM4GBY55BA .

mo22 commented 2 years ago

@baudehlo I have limited time also, but at least I'm using this very module in several projects, so I'd be happy to provide an update and might find time to look through the existing PRs & issues, maybe @paulocoghi or someone else would also be happy to help.

paulocoghi commented 2 years ago

I am focusing on porting all of my NodeJS libraries to JustJS*, and I'm reducing my time on NodeJS projects from now on. Maybe knowing about JustJS will be interesting for you, too.

[*] an ultra-fast javascript engine as fast as C++, based on v8 but without libudev libuv, which makes Javascript the second fastest language on techempower benchmark

[**] also, it allows the generation of an executable much smaller than the ones generated with pkg, with the possibility to remove unused modules from the "standard library" on the final binary

davedoesdev commented 1 year ago

This isn't a bug. The problem is there are too many threads for the UV thread pool, and they're blocked on the file lock. If you increase UV_THREADPOOL_SIZE to e.g. 100 then the program runs to completion. You could also use exnb when locking and retry if you get EAGAIN or EWOULDBLOCK.