coopernurse / node-pool

Generic resource pooling for node.js
2.38k stars 259 forks source link

Consider removing resource is currently not a part of pool errors #203

Open sushantdhiman opened 7 years ago

sushantdhiman commented 7 years ago

Its really easy to call release or destroy on a pooled object which may already have been removed by evictor or some other async userland code.

Pooling library has best knowledge of what resources it currently have, I dont want to check if resource exists in pool before releasing / destroying it.

I might have been missing something, but cant generic-pool ignore destroy / release calls if object is not a part of pool, what was the main reasoning to throw an error in such cases.

sandfox commented 7 years ago

I guess part of the reason for the current pool logic is that I prefer correctness, and if you're passing back already released resources or things that were never part of the pool then something is definitely not correct with the application and you might want to know about it.

If you didn't want to care about it then you could add something like a catch handler that filtered out Resource not currently part of this pool errors. (and yes, that error is slight misnamed because internally it's really caring about the fact the resource isn't loaned out, rather than not belonging to the pool) (I'll fix that next next major version).

That said, I'd accept adding an option to relax the correctness checking (or at least not throw/reject if the user hands the pool something that wasn't currently loaned out)