fukamachi / anypool

General-purpose connection pooling library for Common Lisp
BSD 2-Clause "Simplified" License
25 stars 3 forks source link

Fix: release lock once only #8

Open eternal-turtles opened 5 months ago

eternal-turtles commented 5 months ago

Hi - I noticed a difference in behavior between SBCL and ECL (have only tested these two implementations) in test results for a library that depends on anypool.

On SBCL, the tests for foo.lisp.lack-middleware-redis pass but there is a failure on ECL in which it appears that PUTBACK attempts to release the pool lock more than once.

Test:

  (testing
   "does NOT return 503 response when the maximum open connection count has been reached but pool has availability within the timeout limit"
   (flet ((app (env)
            (declare (ignore env))
            (lack/middleware/redis:with-redis (:mypool)
              (sleep 3)
              (red:set "lack-middleware-redis-test:foo" "foofoo")
              `(200
                (:content-type "text/plain"
                 :content-length 13)
                ("Hello, World.")))))
     (let ((app (funcall lack/middleware/redis:*lack-middleware-redis*
                         #'app
                         :pools '((:pool-id :mypool
                                   :host "localhost"
                                   :port 6379
                                   :max-open-count 2
                                   :max-idle-count 4
                                   :timeout 10000))))
           (failure-lock (bt2:make-lock))
           (failure-count 0))
       (flet ((make-thread ()
                (bt2:make-thread (lambda ()
                                   (let ((response (funcall app ())))
                                     (when (= 503 (first response))
                                       (bt2:with-lock-held (failure-lock)
                                         (incf failure-count)))
                                     response)))))
         (let ((threads (list (make-thread)
                              (make-thread)
                              (make-thread)
                              (make-thread)
                              (make-thread))))
           (dolist (thread threads)
             (bt2:join-thread thread))
           (ok (= 0 failure-count)))))))

On ECL I see the following:

Condition of type: SIMPLE-ERROR
Attempted to give up lock #<lock (nonrecursive) "ANYPOOL-LOCK" 0x7fad8fe0c780> that is not owned by process #<process "Unknown thread 0" 0x7fad933d6d80>
Available restarts:

1. (ABORT) ABORT

Condition of type: SIMPLE-ERROR
Attempted to give up lock #<lock (nonrecursive) "ANYPOOL-LOCK" 0x7fad8fe0c780> that is not owned by process #<process "Unknown thread 1" 0x7fad933d6cc0>
Available restarts:

1. (ABORT) ABORT

I'm unsure if this patch is the right solution, but it does cause the tests to pass on ECL.