Index: lisp/libs/hunchentoot/packages.lisp =================================================================== --- lisp/libs/hunchentoot/packages.lisp (revision 373658) +++ lisp/libs/hunchentoot/packages.lisp (working copy) @@ -43,10 +43,8 @@ (:export "*ACCEPTOR*" "*ACCESS-LOG-PATHNAME*" "*APPROVED-RETURN-CODES*" - #+:lispworks - "*CLEANUP-FUNCTION*" - #+:lispworks - "*CLEANUP-INTERVAL*" + #+:lispworks "*CLEANUP-FUNCTION*" + #+:lispworks "*CLEANUP-INTERVAL*" "*CONTENT-TYPES-FOR-URL-REWRITE*" "*DEFAULT-CONNECTION-TIMEOUT*" "*DEFAULT-CONTENT-TYPE*" @@ -66,7 +64,6 @@ "*METHODS-FOR-POST-PARAMETERS*" "*REPLY*" "*REQUEST*" - "WITHIN-REQUEST-P" "*REWRITE-FOR-SESSION-URLS*" "*SESSION*" "*SESSION-GC-FREQUENCY*" @@ -120,11 +117,10 @@ "+HTTP-USE-PROXY+" "+HTTP-VERSION-NOT-SUPPORTED+" "ABORT-REQUEST-HANDLER" + "ACCEPT-CONNECTIONS" "ACCEPTOR" "ACCEPTOR-ACCESS-LOGGER" "ACCEPTOR-ADDRESS" - "ACCEPT-CONNECTIONS" - "ACCEPTOR-REQUEST-DISPATCHER" "ACCEPTOR-INPUT-CHUNKING-P" "ACCEPTOR-MESSAGE-LOGGER" "ACCEPTOR-NAME" @@ -134,6 +130,7 @@ "ACCEPTOR-READ-TIMEOUT" "ACCEPTOR-REPLY-CLASS" "ACCEPTOR-REQUEST-CLASS" + "ACCEPTOR-REQUEST-DISPATCHER" "ACCEPTOR-SSL-P" #-:hunchentoot-no-ssl "ACCEPTOR-SSL-CERTIFICATE-FILE" #-:hunchentoot-no-ssl "ACCEPTOR-SSL-PRIVATEKEY-FILE" @@ -162,6 +159,8 @@ "CREATE-PREFIX-DISPATCHER" "CREATE-REGEX-DISPATCHER" "CREATE-STATIC-FILE-DISPATCHER-AND-HANDLER" + "CREATE-TASKMASTER-THREAD" + "DECREMENT-TASKMASTER-REQUEST-COUNT" "DEFAULT-DISPATCHER" "DEFINE-EASY-HANDLER" "DELETE-AUX-REQUEST-VALUE" @@ -172,8 +171,8 @@ "GET-PARAMETER" "GET-PARAMETERS" "GET-PARAMETERS*" + "HANDLE-IF-MODIFIED-SINCE" "HANDLE-INCOMING-CONNECTION" - "HANDLE-IF-MODIFIED-SINCE" "HANDLE-STATIC-FILE" "HEADER-IN" "HEADER-IN*" @@ -187,6 +186,7 @@ "HUNCHENTOOT-CONDITION" "HUNCHENTOOT-ERROR" "HUNCHENTOOT-WARNING" + "INCREMENT-TASKMASTER-REQUEST-COUNT" "INITIALIZE-CONNECTION-STREAM" "LOG-MESSAGE" "MIME-TYPE" @@ -224,8 +224,8 @@ "REQUEST-URI*" "REQUIRE-AUTHORIZATION" "RESET-CONNECTION-STREAM" + "RESET-SESSION-SECRET" "RESET-SESSIONS" - "RESET-SESSION-SECRET" "RETURN-CODE" "RETURN-CODE*" "RFC-1123-DATE" @@ -250,8 +250,8 @@ "SET-COOKIE" "SET-COOKIE*" "SHUTDOWN" + #-:hunchentoot-no-ssl "SSL-ACCEPTOR" "SINGLE-THREADED-TASKMASTER" - #-:hunchentoot-no-ssl "SSL-ACCEPTOR" "SSL-P" "START" "START-LISTENING" @@ -259,7 +259,12 @@ "STOP" "TASKMASTER" "TASKMASTER-ACCEPTOR" + "TASKMASTER-MAX-ACCEPT-COUNT" + "TASKMASTER-MAX-THREAD-COUNT" + "TASKMASTER-REQUEST-COUNT" + "TOO-MANY-TASKMASTER-REQUESTS" "URL-DECODE" "URL-ENCODE" - "USER-AGENT")) + "USER-AGENT" + "WITHIN-REQUEST-P")) Index: lisp/libs/hunchentoot/headers.lisp =================================================================== --- lisp/libs/hunchentoot/headers.lisp (revision 375889) +++ lisp/libs/hunchentoot/headers.lisp (working copy) @@ -46,11 +46,10 @@ (and value (regex-replace-all "[\\r\\n]" value " "))) (force-output *header-stream*))) -(defgeneric write-header-line (key value) +(defgeneric write-header-line (key value &optional stream) (:documentation "Accepts a string KEY and a Lisp object VALUE and writes them directly to the client as an HTTP header line.") - (:method (key (string string)) - (let ((stream *hunchentoot-stream*)) + (:method (key (string string) &optional (stream *hunchentoot-stream*)) (labels ((write-header-char (char) (when *header-stream* (write-char char *header-stream*)) @@ -74,9 +73,9 @@ (write-header-char #\Linefeed)) (setf start (1+ end)) (when (<= (length string) start) - (return)))))))) - (:method (key value) - (write-header-line key (princ-to-string value)))) + (return))))))) + (:method (key value &optional (stream *hunchentoot-stream*)) + (write-header-line key (princ-to-string value) stream))) (defun start-output (&key (content nil content-provided-p) (request *request*)) Index: lisp/libs/hunchentoot/taskmaster.lisp =================================================================== --- lisp/libs/hunchentoot/taskmaster.lisp (revision 373658) +++ lisp/libs/hunchentoot/taskmaster.lisp (working copy) @@ -62,6 +62,56 @@ might terminate all threads that are currently associated with it. This function is called by the acceptor's STOP method.")) +(defgeneric create-taskmaster-thread (taskmaster socket) + (:documentation + "Create a new thread in which to process the request. + This thread will call PROCESS-CONNECTION to process the request.")) + +(defgeneric too-many-taskmaster-requests (taskmaster socket) + (:documentation + "Signal a \"too many requests\" error, just prior to closing the connection.")) + +(defgeneric taskmaster-max-thread-count (taskmaster) + (:documentation + "The maximum number of request threads this taskmaster will simultaneously + run before refusing or queueing new connections requests. If the value + is null, then there is no limit.") + (:method ((taskmaster taskmaster)) + "Default method -- no limit on the number of threads." + nil)) + +(defgeneric taskmaster-max-accept-count (taskmaster) + (:documentation + "The maximum number of connections this taskmaster will accept before refusing + new connections. If supplied, this must be greater than MAX-THREAD-COUNT. + The number of queued requests is the difference between MAX-ACCEPT-COUNT + and MAX-THREAD-COUNT.") + (:method ((taskmaster taskmaster)) + "Default method -- no limit on the number of connections." + nil)) + +(defgeneric taskmaster-request-count (taskmaster) + (:documentation + "Returns the current number of taskmaster requests.") + (:method ((taskmaster taskmaster)) + "Default method -- claim there is one connection thread." + 1)) + +(defgeneric increment-taskmaster-request-count (taskmaster) + (:documentation + "Atomically increment the number of taskmaster requests.") + (:method ((taskmaster taskmaster)) + "Default method -- do nothing." + nil)) + +(defgeneric decrement-taskmaster-request-count (taskmaster) + (:documentation + "Atomically decrement the number of taskmaster requests") + (:method ((taskmaster taskmaster)) + "Default method -- do nothing." + nil)) + + (defclass single-threaded-taskmaster (taskmaster) () (:documentation "A taskmaster that runs synchronously in the thread @@ -80,20 +130,144 @@ ;; in a single-threaded environment we just call PROCESS-CONNECTION (process-connection (taskmaster-acceptor taskmaster) socket)) + +(defvar *default-max-thread-count* 100) +(defvar *default-max-accept-count* (+ *default-max-thread-count* 20)) + +;; You might think it would be nice to provide a taskmaster that takes +;; threads out of a thread pool. There are two things to consider: +;; - On a 2010-ish Linux box, thread creation takes less than 250 microseconds. +;; - Bordeaux Threads doesn't provide a way to "reset" and restart a thread, +;; and it's not clear how many Lisp implementations can do this. +;; So for now, we leave this out of the mix. (defclass one-thread-per-connection-taskmaster (taskmaster) (#-:lispworks - (acceptor-process :accessor acceptor-process - :documentation "A process that accepts incoming -connections and hands them off to new processes for request -handling.")) + (acceptor-process + :accessor acceptor-process + :documentation + "A process that accepts incoming connections and hands them off to new processes + for request handling.") + ;; Support for bounding the number of threads we'll create + (max-thread-count + :type (or integer null) + :initarg :max-thread-count + :initform nil + :accessor taskmaster-max-thread-count + :documentation + "The maximum number of request threads this taskmaster will simultaneously + run before refusing or queueing new connections requests. If the value + is null, then there is no limit.") + (max-accept-count + :type (or integer null) + :initarg :max-accept-count + :initform nil + :accessor taskmaster-max-accept-count + :documentation + "The maximum number of connections this taskmaster will accept before refusing + new connections. If supplied, this must be greater than MAX-THREAD-COUNT. + The number of queued requests is the difference between MAX-ACCEPT-COUNT + and MAX-THREAD-COUNT.") + (request-count + :type integer + :initform 0 + :accessor taskmaster-request-count + :documentation + "The number of taskmaster threads currently running.") + (request-count-lock + :initform (bt:make-lock "taskmaster-request-count") + :reader taskmaster-request-count-lock + :documentation + "In the absence of 'atomic-incf', we need this to atomically + increment and decrement the request count.") + (wait-queue + :initform (bt:make-condition-variable) + :reader taskmaster-wait-queue + :documentation + "A queue that we use to wait for a free connection.") + (wait-lock + :initform (make-lock "taskmaster-thread-lock") + :reader taskmaster-wait-lock + :documentation + "The lock for the connection wait queue.") + (worker-thread-name-format + :type (or string null) + :initarg :worker-thread-name-format + :initform "hunchentoot-worker-~A" + :accessor taskmaster-worker-thread-name-format)) + (:default-initargs + :max-thread-count *default-max-thread-count* + :max-accept-count *default-max-accept-count*) (:documentation "A taskmaster that starts one thread for listening -to incoming requests and one thread for each incoming connection. +to incoming requests and one new thread for each incoming connection. +If MAX-THREAD-COUNT is null, a new thread will always be created for +each request. + +If MAX-THREAD-COUNT is supplied, the number of request threads is +limited to that. Furthermore, if MAX-ACCEPT-COUNT is not supplied, an +HTTP 503 will be sent if the thread limit is exceeded. Otherwise, if +MAX-ACCEPT-COUNT is supplied, it must be greater than MAX-THREAD-COUNT; +in this case, requests are accepted up to MAX-ACCEPT-COUNT, and only +then is HTTP 503 sent. + +In a load-balanced environment with multiple Hunchentoot servers, it's +reasonable to provide MAX-THREAD-COUNT but leave MAX-ACCEPT-COUNT null. +This will immediately result in HTTP 503 when one server is out of +resources, so the load balancer can try to find another server. + +In an environment with a single Hunchentoot server, it's reasonable +to provide both MAX-THREAD-COUNT and a somewhat larger value for +MAX-ACCEPT-COUNT. This will cause a server that's almost out of +resources to wait a bit; if the server is completely out of resources, +then the reply will be HTTP 503. + This is the default taskmaster implementation for multi-threaded Lisp implementations.")) -;; usocket implementation +(defmethod initialize-instance :after ((taskmaster one-thread-per-connection-taskmaster) &rest init-args) + "Ensure the if MAX-ACCEPT-COUNT is supplied, that it is greater than MAX-THREAD-COUNT." + (declare (ignore init-args)) + (when (taskmaster-max-accept-count taskmaster) + (unless (taskmaster-max-thread-count taskmaster) + (parameter-error "MAX-THREAD-COUNT must be supplied if MAX-ACCEPT-COUNT is supplied")) + (unless (> (taskmaster-max-accept-count taskmaster) (taskmaster-max-thread-count taskmaster)) + (parameter-error "MAX-ACCEPT-COUNT must be greater than MAX-THREAD-COUNT")))) +(defmethod increment-taskmaster-request-count ((taskmaster one-thread-per-connection-taskmaster)) + (when (taskmaster-max-thread-count taskmaster) + (bt:with-lock-held ((taskmaster-request-count-lock taskmaster)) + (incf (taskmaster-request-count taskmaster))))) + +(defmethod decrement-taskmaster-request-count ((taskmaster one-thread-per-connection-taskmaster)) + (when (taskmaster-max-thread-count taskmaster) + (prog1 + (bt:with-lock-held ((taskmaster-request-count-lock taskmaster)) + (decf (taskmaster-request-count taskmaster))) + (when (and (taskmaster-max-accept-count taskmaster) + (< (taskmaster-request-count taskmaster) (taskmaster-max-accept-count taskmaster))) + (note-free-connection taskmaster))))) + +(defmethod note-free-connection ((taskmaster one-thread-per-connection-taskmaster)) + "Note that a connection has been freed up" + (bt:with-lock-held ((taskmaster-wait-lock taskmaster)) + (bt:condition-notify (taskmaster-wait-queue taskmaster)))) + +(defmethod wait-for-free-connection ((taskmaster one-thread-per-connection-taskmaster)) + "Wait for a connection to be freed up" + (bt:with-lock-held ((taskmaster-wait-lock taskmaster)) + (loop until (< (taskmaster-request-count taskmaster) (taskmaster-max-thread-count taskmaster)) + do (bt:condition-wait (taskmaster-wait-queue taskmaster) (taskmaster-wait-lock taskmaster))))) + +(defmethod too-many-taskmaster-requests ((taskmaster one-thread-per-connection-taskmaster) socket) + (declare (ignore socket)) + (let* ((acceptor (taskmaster-acceptor taskmaster)) + (logger (and acceptor (acceptor-message-logger acceptor)))) + (when logger + (funcall logger :warning "Can't handle a new request, too many request threads already")))) + + +;;; usocket implementation + #-:lispworks (defmethod shutdown ((taskmaster taskmaster)) taskmaster) @@ -110,16 +284,76 @@ #-:lispworks (defmethod execute-acceptor ((taskmaster one-thread-per-connection-taskmaster)) (setf (acceptor-process taskmaster) - (bt:make-thread (lambda () + (bt:make-thread + (lambda () (accept-connections (taskmaster-acceptor taskmaster))) - :name (format nil "Hunchentoot listener \(~A:~A)" + :name (format nil "hunchentoot-listener-~A:~A" (or (acceptor-address (taskmaster-acceptor taskmaster)) "*") (acceptor-port (taskmaster-acceptor taskmaster)))))) #-:lispworks +(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) + ;; Here's the idea, with the stipulations given in ONE-THREAD-PER-CONNECTION-TASKMASTER + ;; - If MAX-THREAD-COUNT is null, just start a taskmaster + ;; - If the connection count will exceed MAX-ACCEPT-COUNT or if MAX-ACCEPT-COUNT + ;; is null and the connection count will exceed MAX-THREAD-COUNT, + ;; return an HTTP 503 error to the client + ;; - Otherwise if we're between MAX-THREAD-COUNT and MAX-ACCEPT-COUNT, + ;; wait until the connection count drops, then handle the request + ;; - Otherwise, increment REQUEST-COUNT and start a taskmaster + (cond ((null (taskmaster-max-thread-count taskmaster)) + ;; No limit on number of requests, just start a taskmaster + (create-taskmaster-thread taskmaster socket)) + ((if (taskmaster-max-accept-count taskmaster) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-accept-count taskmaster)) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-thread-count taskmaster))) + ;; Send HTTP 503 to indicate that we can't handle the request right now + (too-many-taskmaster-requests taskmaster socket) + (send-http-error-reply taskmaster socket +http-service-unavailable+)) + ((and (taskmaster-max-accept-count taskmaster) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-thread-count taskmaster))) + ;; Wait for a request to finish, then carry on + (wait-for-free-connection taskmaster) + (increment-taskmaster-request-count taskmaster) + (create-taskmaster-thread taskmaster socket)) + (t + ;; We're within both limits, just start a taskmaster + (increment-taskmaster-request-count taskmaster) + (create-taskmaster-thread taskmaster socket)))) + +(defun send-http-error-reply (taskmaster socket error-code) + "A helper function to send out a quick error reply, + before any state is set up via PROCESS-REQUEST." + (let* ((acceptor (taskmaster-acceptor taskmaster)) + (stream (initialize-connection-stream acceptor (make-socket-stream socket acceptor))) + (reason-phrase (reason-phrase error-code)) + (first-line (format nil "HTTP/1.1 ~D ~A" + error-code reason-phrase)) + (content (format nil "~D ~A

~:*~A

~A" + error-code reason-phrase reason-phrase))) + (write-sequence (map 'list #'char-code first-line) stream) + (write-sequence +crlf+ stream) ;end of first line + (write-header-line "Content-Type" "text/html; charset=iso-8859-1" stream) + (write-header-line "Content-Length" (length content) stream) + (write-sequence +crlf+ stream) ;end of headers + (write-sequence (map 'list #'char-code content) stream) + (write-sequence +crlf+ stream) ;end of content + (force-output stream))) + +#-:lispworks +(defmethod create-taskmaster-thread ((taskmaster one-thread-per-connection-taskmaster) socket) + "Create a thread for handling a single request" + (bt:make-thread + (lambda () + (multiple-value-prog1 + (process-connection (taskmaster-acceptor taskmaster) socket) + (decrement-taskmaster-request-count taskmaster))) + :name (format nil (taskmaster-worker-thread-name-format taskmaster) (client-as-string socket)))) + +#-:lispworks (defun client-as-string (socket) "A helper function which returns the client's address and port as a -string and tries to act robustly in the presence of network problems." + string and tries to act robustly in the presence of network problems." (let ((address (usocket:get-peer-address socket)) (port (usocket:get-peer-port socket))) (when (and address port) @@ -127,13 +361,8 @@ (usocket:vector-quad-to-dotted-quad address) port)))) -#-:lispworks -(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) - (bt:make-thread (lambda () - (process-connection (taskmaster-acceptor taskmaster) socket)) - :name (format nil "Hunchentoot worker \(client: ~A)" (client-as-string socket)))) -;; LispWorks implementation +;;; LispWorks implementation #+:lispworks (defmethod shutdown ((taskmaster taskmaster)) @@ -148,15 +377,39 @@ (accept-connections (taskmaster-acceptor taskmaster))) #+:lispworks -(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) handle) +(defmethod handle-incoming-connection ((taskmaster one-thread-per-connection-taskmaster) socket) (incf *worker-counter*) ;; check if we need to perform a global GC (when (and *cleanup-interval* (zerop (mod *worker-counter* *cleanup-interval*))) (when *cleanup-function* (funcall *cleanup-function*))) - (mp:process-run-function (format nil "Hunchentoot worker \(client: ~{~A:~A~})" + (cond ((null (taskmaster-max-thread-count taskmaster)) + ;; No limit on number of requests, just start a taskmaster + (create-taskmaster-thread taskmaster socket)) + ((if (taskmaster-max-accept-count taskmaster) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-accept-count taskmaster)) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-thread-count taskmaster))) + ;; Send HTTP 503 to indicate that we can't handle the request right now + (too-many-taskmaster-requests taskmaster socket) + (send-http-error-reply taskmaster socket +http-service-unavailable+)) + ((and (taskmaster-max-accept-count taskmaster) + (>= (taskmaster-request-count taskmaster) (taskmaster-max-thread-count taskmaster))) + ;; Lispworks doesn't have condition variables, so punt + (too-many-taskmaster-requests taskmaster socket) + (send-http-error-reply taskmaster socket +http-service-unavailable+)) + (t + ;; We're within both limits, just start a taskmaster + (increment-taskmaster-request-count taskmaster) + (create-taskmaster-thread taskmaster socket)))) + +#+:lispworks +(defmethod create-taskmaster-thread ((taskmaster one-thread-per-connection-taskmaster) socket) + (flet ((process (taskmaster sock) + (multiple-value-prog1 + (process-connection (taskmaster-acceptor taskmaster) socket) + (decrement-taskmaster-request-count taskmaster)))) + (mp:process-run-function (format nil "hunchentoot-worker-{~A:~A~})" (multiple-value-list - (get-peer-address-and-port handle))) - nil #'process-connection - (taskmaster-acceptor taskmaster) handle)) + (get-peer-address-and-port socket))) + nil #'process taskmaster socket))) Index: lisp/libs/hunchentoot/acceptor.lisp =================================================================== --- lisp/libs/hunchentoot/acceptor.lisp (revision 373658) +++ lisp/libs/hunchentoot/acceptor.lisp (working copy) @@ -117,8 +117,7 @@ process different from the one where START was called.") #-:lispworks (listen-socket :accessor acceptor-listen-socket - :documentation "The socket listening for incoming -connections.") + :documentation "The socket listening for incoming connections.") (acceptor-shutdown-p :initform nil :accessor acceptor-shutdown-p :documentation "A flag that makes the acceptor @@ -349,8 +348,9 @@ ;; the default is to always answer "no" nil) -;; usocket implementation +;;; usocket implementation + #-:lispworks (defmethod start-listening ((acceptor acceptor)) (setf (acceptor-listen-socket acceptor) @@ -373,13 +373,14 @@ (set-timeouts client-connection (acceptor-read-timeout acceptor) (acceptor-write-timeout acceptor)) - (handle-incoming-connection (acceptor-taskmaster acceptor) - client-connection)) - ;; ignore condition + ;; This will bail if the taskmaster has reached its thread limit + (handle-incoming-connection (acceptor-taskmaster acceptor) client-connection)) + ;; Ignore the error (usocket:connection-aborted-error ())))))) -;; LispWorks implementation +;;; LispWorks implementation + #+:lispworks (defmethod start-listening ((acceptor acceptor)) (multiple-value-bind (listener-process startup-condition) @@ -399,7 +400,7 @@ :function (lambda (handle) (unless (acceptor-shutdown-p acceptor) (handle-incoming-connection - (acceptor-taskmaster acceptor) handle))) + (acceptor-taskmaster acceptor) client-connection))) ;; wait until the acceptor was successfully started ;; or an error condition is returned :wait t) @@ -414,6 +415,7 @@ (mp:process-unstop (acceptor-process acceptor)) nil) + (defun list-request-dispatcher (request) "The default request dispatcher which selects a request handler based on a list of individual request dispatchers all of which can