edicl / hunchentoot

Web server written in Common Lisp
https://edicl.github.io/hunchentoot/
701 stars 124 forks source link

Will server stop softly if two request processing at same time? #141

Open imnisen opened 6 years ago

imnisen commented 6 years ago

Hi, when I read code about how server stop softly, I have a quesion:

;; acceptor.lisp  line318
(defmethod stop ((acceptor acceptor) &key soft)
  (with-lock-held ((acceptor-shutdown-lock acceptor))
    (setf (acceptor-shutdown-p acceptor) t))
  #-lispworks
  (wake-acceptor-for-shutdown acceptor)
  (when soft
    (with-lock-held ((acceptor-shutdown-lock acceptor))
      (when (plusp (accessor-requests-in-progress acceptor))
        (condition-variable-wait (acceptor-shutdown-queue acceptor)     ;<---here
                                 (acceptor-shutdown-lock acceptor)))))
  (shutdown (acceptor-taskmaster acceptor))
  ...)

when soft is set t and (accessor-requests-in-progress acceptor) is greater than 0, here use condition-variable-wait to wait on (acceptor-shutdown-queue acceptor).

And the condition signal happens in process-connection when handling each incoming connection if acceptor is shutdown, as following code:

;; acceptor.lisp  line370
(defun do-with-acceptor-request-count-incremented (*acceptor* function)
  (with-lock-held ((acceptor-shutdown-lock *acceptor*))
    (incf (accessor-requests-in-progress *acceptor*)))
  (unwind-protect
       (funcall function)
    (with-lock-held ((acceptor-shutdown-lock *acceptor*))
      (decf (accessor-requests-in-progress *acceptor*))
      (when (acceptor-shutdown-p *acceptor*)
        (condition-variable-signal (acceptor-shutdown-queue *acceptor*))))))   ;<---here

If I don't misunderstand it, in mutli-threads server, when two threads handle two request at same time, then make a call of server stop with parameter soft set to true, then this first one finish the request will call condition-variable-signal, and acceptor will execute the following (shutdown (acceptor-taskmaster acceptor) showdown code. But it will not wait the second request to finish.

May I got it wrong? Why work like this?

Thank you

CloseToZero commented 3 years ago

If I don't misunderstand it, in mutli-threads server, when two threads handle two request at same time, then make a call of server stop with parameter soft set to true, then this first one finish the request will call condition-variable-signal, and acceptor will execute the following (shutdown (acceptor-taskmaster acceptor) showdown code. But it will not wait the second request to finish.

Probably not, the server will respond correctly even there are multiple remaining client connections.

As a test case, define the following two handlers:

(define-easy-handler (h1 :uri "/h1") ()
  (sleep 10)
  (format nil "h1"))

(define-easy-handler (h2 :uri "/h2") ()
  (sleep 20)
  (format nil "h2"))

Then access the two URLs in a browser and evaluate (stop your-acceptor :soft t) within 10 seconds. Later, you will see them both get the response correctly.

Reason:

After you evaluate (stop your-acceptor :soft t), stop method will wait until the first worker thread signals the condition variable then execute (shutdown (acceptor-taskmaster acceptor)) (Note: the only thing shutdown method does is wait until the accept thread stopped), it will not wait for other worker threads.

After the shutdown method returns, stop will call (usocket:socket-close (acceptor-listen-socket acceptor)) to close the listening socket but keep other connected sockets untouched, other connected sockets will function correctly even after the listening socket closed, that's the reason why both clients get the response correctly.

You can modify the stop method as following to check that stop will only wait for the first worker thread:

(with-lock-held ((acceptor-shutdown-lock acceptor))
  (when (plusp (acceptor-requests-in-progress acceptor))
    (condition-variable-wait (acceptor-shutdown-queue acceptor)
                             (acceptor-shutdown-lock acceptor))
    (format t "finish!!!")))

The question that remains is: hunchentoot need not even wait for the first worker thread stopped, it can close the listening socket directly.