Hunchentoot: acceptor code review

This post is part of a series on Common Lisp WWWism, more specifically of ongoing work to understand the web server known as Hunchentoot. Even more specifically, this post exhaustively covers each of the methods and functions implemented by the acceptor class.

Let's begin with my own version of (ac)counting the functions in acceptor.lisp. First, we read all the top-level S-expressions in the file:

(defparameter *top-level-sexprs*
  (with-open-file (in "acceptor.lisp")
       (loop for sexpr = (read in nil :err)
        until (eq sexpr :err)
        collect sexpr)))

and after fiddling around with fixing read-time weird encounters, we keep expressions that are lists beginning with "defmethod" or "defun":

(defparameter *funcs*
  (loop for def in *top-level-sexprs*
       with acc = nil do
       (when (and (consp def) (member (car def) '(defmethod defun)))
     (push (list (car def) (cadr def)) acc))
     finally (return (nreverse acc))))

Then we:

> (length *funcs*)
27

which is weird, because counting them by hand (for the purpose of e.g. determining the number of lines occupied by each function) yields no less than 30; which actually makes sense, because some of the definitions have a "#+lispworks" in front of them, which causes the CL reader to ignore them. So the first observation is that the Hunchentoot coad is full of ifdefism of the lowest kind. And since I don't have a Lispworks and I'm not planning to support it, my branch of the Hunchentoot V tree will be devoid of any "#+lispworks", and ultimately of any "#+..." whatsoever.

The second observation is that most methods -- notable exceptions being process-connection and acceptor-status-message -- are actually quite small and easy to read; most of the gnarl lies in heavy "systems code" that has to do corner-case handling for some particularity of e.g. the HTTP protocol, and in some cases even that could be refactored into something sane. And now without further ado, we dive straight into the function/method definitions. Note the organization: <anchor> <function name with link to source code>: <description>, which should help the reader to more easily navigate this maze of references.

[ddd] default-document-directory: Function for finding the "default document root", i.e. the "www/" directory in the Hunchentoot coad base. This directory contains a test page and the pre-generated Hunchentoot documentation (otherwise found in "doc"). I'm not convinced this is really needed, other than perhaps for a quick test of new Hunchentoot installations. Then again, any other filesystem path that the operator desires could be set as a document root for that purpose.

[po] print-object: Implementation for the printing method, called when e.g. "print" is called on an acceptor object.

[ii] initialize-instance: method for initializing acceptor objects, called when "make-instance" is called. Note the ":after" method qualifier, specifying that this implementation of initialize-instance is called after that of the superclass.

This defmethod: a. sets the persistent-connections-p field to nil for single-threaded taskmasters; and b. sets the document root and error template directory to their so-called physical pathname. In particular (b) allows using so-called "logical pathnames", which I'll admit I never used, but which supposedly aim to implement a sort of pseudo-GNS for refering to files using filesystem-agnostic resource locators.

[s] start: a. sets "self" acceptor as the acceptor of its current taskmaster (why isn't this being done in initialize-instance?); b. calls start-listening; c. calls the taskmaster's execute-acceptor.

[s2] stop: a. sets acceptor-shutdown-p to true; b. calls wake-acceptor-for-shutdown; c. when "soft" is set to true, waits on the shutdown-queue condition variable for the next outstanding request to be served; d. calls the taskmaster's shutdown method; e. closes and loses the reference to the acceptor's listening socket, thus allowing it to be garbage collected.

Note on (c): while I'm not sure this "soft shutdown" mechanism is of much use at all¹, we observe that it escapes the problem of indefinite postponment by waiting on only the first in-progress request to be served. That is, if the server wanted to play really nicely, it would wait on that condition variable until requests-in-progress went down to zero; however, in this case some really mean fucker who does not want to let us finish could have generated requests ad infinitum, or in any case, until the next power outage. Fortunately, whoever designed this wasn't that naive.

[wafs] wake-acceptor-for-shutdown: "Creates a dummy connection to the acceptor, waking accept-connections while it is waiting. This is supposed to force a check of acceptor-shutdown-p." And you have to admit, that's a pretty weird way of forcing the check too, but it works; anyway, a much simpler approach than using CFFI and epoll.

[ics] initialize-connection-stream: By default it returns the stream object as it was passed to it (and naught else), but sub-acceptors can override/extend this behaviour. Off the top of my head I'm not sure why I'd need to implement a sub-method of this myself, for now it's just that the "SSL acceptor" (which is well on the way to extinction) uses it to do SSListic stuff.

[rcs] reset-connection-stream: "Resets the stream which is used to communicate between client and server after one request has been served so that it can be used to process the next request." We're not told what this "reset" means more exactly, so we're stuck trying to reverse it from the context. Note that similarly to the previous method, this can be extended by users to perform various actions on streams; only it's not called at the very beginning of a connection, but after each request.

Our reset-connection-stream function is called from process-connection after processing the current request and flushing the stream (why is "flushing" a separate operation from "resetting" anyway? no idea). One would expect thusly that "resetting" would have something to do with "request processing", only there's no apparent relation between the two: the method implementation only turns the connection stream from a "chunked stream" (assuming chunking² is enabled, which it is, by default) to a "non-chunked" (regular) socket stream. I'm not sure exactly why this is done at this point, since chunking is later re-enabled either by the server and/or client upon response/request delivery.

[dwarci] do-with-acceptor-request-count-incremented: a. atomically increments requests-in-progress; b. calls a function received as a parameter; c. decrements requests-in-progress; d. if acceptor-shutdown-p is set, then it signals a wake-up through the shutdown-queue.

This is a reference-counting thing used to keep track of the number of requests in the progress of being served at any given time. It's used exclusively in conjunction with process-request.

[warci] with-acceptor-request-count-incremented: Wrapper around the previous function, which conveniently allows creating a context for executing code with said fields incremented.

[amr] acceptor-make-request: Given all request parameters (headers, "content stream", HTTP method, URI and HTTP protocol version), plus the remote and local addresses taken from the socket, instantiates a new request object.

[ds] detach-socket: This isn't called from, nor subclassed, anywhere. Judging by the fact that it sets *finish-processing-socket* (to true) and *close-hunchentoot-stream* (to nil), it's apparent that it signals to the (calling) process-connection that it should stop doing any further processing on the socket of the current connection and "abandon" it, i.e. hand over control over it to the user.

If there is an honest use case for this, I don't know it yet.

[pc] process-connection: This is a fucking huge beast, moreso that it calls most of the methods and functions defined above. Let's look at it.

There's also an ":around"-qualified method defined at line 363, which does nothing but wrap a call-next-method call in a context that ensures that all errors and warnings are logged by the server. Thus all other method implementations, including the one at 427 are run in this error handling context.

As for the main implementation, it: a. takes the current acceptor socket stream; b. initializes it; then c. runs a request processing loop (which we'll get to immediately); and d. if *close-hunchentoot-stream* is set, then it flushes and closes the socket stream under processing.

As for the processing loop, it: 1. checks whether it needs to shut down, in which case it ends; 2. calls get-request-data³, returning headers, URL, method and protocol version; 3. checks that there is a valid method; 4. does some gnarly handling for "transfer encodings", i.e. chunking, then 5. with-acceptor-request-count-incremented it calls process-request on a new request generated using acceptor-make-request; 6. it flushes the output stream; 7. resets it; and then finally, 8. if *finish-processing-socket* is set, it returns.

If you've been following closely, you'll see that the thing only looks horrible and that it otherwise makes sense for the most part, plus all the complexity involved in chunking, tracking request counts and the "socket abandoning" described above. All in all, a good candidate for refactoring, altough to be honest, if you wake me up tonight and ask me to do that, I won't know where to start.

[asp2] acceptor-ssl-p: This one's well on the way out.

[ala] acceptor-log-access: logs page accesses. Called upon sending a response to the client.

[alm] acceptor-log-message: Similar to the previous function, only it logs to message-log-destination. If you're familiar with Apache, then these are very similar to the AccessLog and ErrorLog facilities. Called from the taskmaster and from the following wrapper function.

[lms] log-message*: Convenient wrapper for the previous method, that logs for the currently bound *acceptor* special variable. This is the message logger used all around the Hunchentoot code.

[sl] start-listening: When the given acceptor isn't listening, create a new listening socket; call usocket's socket-listen, which does all the binding etc.

An ":after" method is also implemented, which sets the acceptor's port if it was set to zero, i.e. it was allocated by the operating system and at that point it's setting its actual value.

[ac] accept-connections: In a loop: a. if acceptor-shutdown-p is set, then return; otherwise b. wait-for-input, then c. socket-accept on the listening socket; d. set-timeouts for new socket; e. schedule connection handling for the new socket via taskmaster's handle-incoming-connection.

accept-connections is called by the taskmaster on execute-acceptor.

[adp] acceptor-dispatch-request: As previously discussed: a. if the document-root exists, then a static file is served; otherwise, b. a HTTP not-found is returned.

In particular, (a) calls handle-static-file; let's look at this one in more detail: it 1. checks that the file exists (using FAD) and replies with a HTTP not-found if it doesn't find it; 2. sets the content-type using mime-type⁴; 3. checks whether the file has been modified since some particular time specified in the request; 4. attempts to handle the range header, if it exists; 5. sends the headers; 6. in a loop, sends chunks of data while there are bytes available to send; and 7. flushes the output stream before exiting.

In particular, (b) calls abort-request-handler, which transfers control to handler-done, i.e. abruptly ends request handling and sends the reply down the output stream.

[hr] handle-request: called from within the "request processing" context; it wraps acceptor-dispatch-request in a condition catcher that recovers from any errors that might occur in the process of handling requests.

[asm] acceptor-status-message: Used by Hunchentoot to "compose" "status replies", e.g. 40x pages, served from e.g. process-request. This one's another beast, so I'm breaking it into as-small-as-possible pieces to ingurgitate. There's more than one definition for this method: the main one (736), a "default" one (721) and an ":around"-qualified one (724) -- let's take them one by one.

The ":around" method runs the first thing when acceptor-status-message is called; it calls the next method, wrapped in an error handling context that logs the error and falls back to make-cooked-message.

The "default" method is the one that gets called when all other calls fail -- notice the "(acceptor t)" specialization, t being the mother of all types. Similarly to the error handling case above, this one calls make-cooked-message.

The main method: a. defines some local procedures (which we'll examine below); b. ignores status codes smaller than 300⁵; and c. it calls one of the local procedures, error-contents-from-template, which tests whether a file "xxx.html" (where xxx is the status code) exists⁶ in the error-template-directory, and if the case, it serves it under a "request context variable" substitution. In order to serve the file, its contents are read using the file-contents local procedure and then the so-called variable substitution is performed using the substitute-request-context-variables local procedure.

file-contents reads the entire contents of the file in a buffer using read-sequence.

substitute-request-context-variables looks for variables between curly braces preceded by a dollar, such as, say, ${script-name} and then substitutes them according to the contents of "properties". Not sure it's obvious by now, but I believe this ad-hoc "context variable substitution" to be a terrible idea. I genesized cl-who for precisely this type of thing and I'm not going to have it replaced by some half-assed pseudo-solution concocted by Dorel in one of his sleepless nights⁷.

Anyway, from the acceptor-status-message series, we're left with:

[mcm] make-cooked-message: Given a HTTP status code and optional Lisp error and backtrace objects, it formats an Apache-style error page containing a short description of the status code. The bulk of this formatting occurs in the cooked-message local procedure, while the function body matches the HTTP code to the description message and calls cooked-message.

[sak] string-as-keyword: Used by acceptor-status-message to convert a string (denoting the name of something) into a symbol from the keyword package.

[ars] acceptor-remove-session: Used downstream for session cleanup, in remove-session and, I suspect, in session management code written by the user. At this level, the implementation doesn't do anything so it must exist mainly to be subclassed by users.

[asn] acceptor-server-name: "Returns a string which can be used for 'Server' headers." Clear enough from where I'm standing.

This is it for now. I only have three other fundamental Hunchentoot pieces to dissect in this manner, plus, as can be observed here, some of the auxiliary code.

Meta: given that proper in-blog comments are for now still missing, readers are invited to leave them in #spyked or (assuming they know where they're heading) #trilema. Existing comments are preserved here in ⁸.