search

coopernurse / node-pool

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

destroying with "in use" connections can trap connections forever #122

Closed jasonrhodes closed 8 years ago

jasonrhodes commented 8 years ago

Note: this bug is already fixed in v2.4.0, but it's not called out anywhere in issues or mentioned in the Changelog, so this is for reference in case others run into the problem.

We noticed that after upgrading to v2.3.1 from v2.2.0, after some time connections would pile up. After much debugging, we realized that a call to destroy while there are connection objects in the inUseObjects would inadvertantly clear all of those in use objects, leaving them connected but making them un-releasable. (For us, this happened when a connection was invalidated via the validate function, but while other connections were still in use, so only occasionally. Would take about 30-40 min to manifest while making 10 req/second.)

inUseObjects = inUseObjects.filter(function(obj) {
  return (obj !== obj);
});

It's supposed to get rid of the passed in object if it happens to be in the inUseObjects array, but instead clears them all out no matter what because of obj !== obj. This led to amassing connections and eventually 500ing our APIs until we hard restarted, clearing all the trapped connections.

The bug is introduced in 7eef89d130c033c69cf3dfc0fbf28d29e4ae7fd0 and released in v2.2.2, and was subsequently fixed with b97f07262e4142bf78bc83340a0becdcfa4bae46 which is a commit doing lint cleanups, but doesn't mention this bugfix. The release went out in v2.4.0.

PR incoming to make a note of this fix in the Changelog, for future reference.

sandfox commented 8 years ago

Awesome debugging work and thanks for the PR! (and sorry for the bug in the first place :cry: ). I'll and try and get this merged + released in with next several days.