Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unref setTimeout in pool #300

Merged
merged 1 commit into from Aug 4, 2022
Merged

Conversation

nomagick
Copy link
Contributor

@nomagick nomagick commented Aug 3, 2022

I'm using generic-pool in another library.
The end developer may not be aware there's a pool running under the hood.
Thus unable to drain().then(clear()).

As a library, don't block the process quit.
Simply unref all the setTimeouts.

As a library don't block the process quit. Simply `unref` all the `setTimeout`s.
@Kikobeats Kikobeats merged commit 6a88c64 into coopernurse:master Aug 4, 2022
@nomagick nomagick deleted the patch-2 branch August 4, 2022 09:51
@tal952
Copy link

tal952 commented May 21, 2023

This throws exception in react:

Uncaught TypeError: setTimeout(...).unref is not a function

@nomagick
Copy link
Contributor Author

This throws exception in react:

Uncaught TypeError: setTimeout(...).unref is not a function

u running this in browser or node.js or some other javascript runtime?

@tal952
Copy link

tal952 commented May 22, 2023

In chrome, yes

@nomagick
Copy link
Contributor Author

This project is node-pool and Generic resource pooling for node.js
So I think it's ok to rely on node.js only features.

setTimeout returns a timer in node.js, while in browsers, it returns an integer.

I guess u need to fork and change the code for if you need this library in the browser.

Simply remove the .unref part cuz in browsers u don't need to worry about timer references and quitting the process.

@tal952
Copy link

tal952 commented May 22, 2023

Actually, in npm, it is called 'generic-pool', and there is no statement that says it's exclusively for Node.js. Is there anything that makes the library Node.js only? Shouldn't we just change the call to .unref?.() and support browsers?

@nomagick
Copy link
Contributor Author

Actually, in npm, it is called 'generic-pool'

@tal952 I see. I created another PR for this. It's not tested. I just modified the code in my browser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants