I'm not certain whether this is considered a bug in Knex, Tarn, or my code, but I'll start here.
I noticed that one of our programs sometimes timed out on exit. From what I can tell via debugging, it happens if the program attempts to exit early while certain database operations are going on asynchronously. This can result in one of the not-yet-canceled async operations calling acquire after destroy is called. The problem appears to be caused by the interaction of Tarn's Pool.acquire function and _tryAcquireOrCreate functions: _tryAcquireOrCreate exits early if the pool has been destroyed, but that only occurs after acquire has already created the pendingAcquire timeout, so the program appears to hang until that acquire timeout has elapsed.
Possible solutions:
PendingOperation's timeout calls unref on its timer so that a pending operation timeout by itself can't keep the Node.js runtime alive.
Pool.acquire checks this.destroyed before doing anything. If the pool has been destroyed, it rejects with an appropriate error.
Unref'ing the timer could risk unwanted effects, but adding a destroyed check to acquire seems reasonable and consistent.
I'm not certain whether this is considered a bug in Knex, Tarn, or my code, but I'll start here.
I noticed that one of our programs sometimes timed out on exit. From what I can tell via debugging, it happens if the program attempts to exit early while certain database operations are going on asynchronously. This can result in one of the not-yet-canceled async operations calling
acquireafterdestroyis called. The problem appears to be caused by the interaction of Tarn'sPool.acquirefunction and_tryAcquireOrCreatefunctions:_tryAcquireOrCreateexits early if the pool has been destroyed, but that only occurs afteracquirehas already created thependingAcquiretimeout, so the program appears to hang until that acquire timeout has elapsed.Possible solutions:
PendingOperation's timeout calls unref on its timer so that a pending operation timeout by itself can't keep the Node.js runtime alive.Pool.acquirechecksthis.destroyedbefore doing anything. If the pool has been destroyed, it rejects with an appropriate error.Unref'ing the timer could risk unwanted effects, but adding a
destroyedcheck to acquire seems reasonable and consistent.