- Improve documentation for the MessageChannel and SpawnManager C++ c…

…lasses.

- Make sure a 500 Internal Server Error is returned if an unexpected exception is caught during a HTTP request.
- Make the spawn server print its errors to the Apache error log file instead of its own log file.

ext/apache2/Hooks.cpp

@@ -217,8 +217,7 @@ class Hooks {
 		ap_add_version_component(pconf, "Phusion_Passenger/" PASSENGER_VERSION);
 
 		const char *spawnManagerCommand = "/home/hongli/Projects/mod_rails/lib/mod_rails/spawn_manager.rb";
-		const char *logFile = "/home/hongli/Projects/mod_rails/spawner_log.txt";
-		applicationPool = ApplicationPoolPtr(new ApplicationPool(spawnManagerCommand, logFile, "production"));
+		applicationPool = ApplicationPoolPtr(new ApplicationPool(spawnManagerCommand, "", "production"));
 	}
 
 	~Hooks() {
@@ -229,7 +228,6 @@ class Hooks {
 	}
 
 	int handleRequest(request_rec *r) {
-		// The main request handler hook function.
 		RailsConfig *config = getConfig(r);
 		const char *railsDir;
 
@@ -254,8 +252,6 @@ class Hooks {
 			return OK;
 		}
 
-
-
 		/* int httpStatus = ap_setup_client_block(r, REQUEST_CHUNKED_ERROR);
     		if (httpStatus != OK) {
 			return httpStatus;
@@ -280,10 +276,12 @@ class Hooks {
 
 			ap_scan_script_header_err_brigade(r, bb, NULL);
 			ap_pass_brigade(r->output_filters, bb);
+
+			return OK;
 		} catch (const exception &e) {
 			ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, "mod_passenger unknown error: %s", e.what());
+			return HTTP_INTERNAL_SERVER_ERROR;
 		}
-		return OK;
 	}
 
 	int

ext/apache2/MessageChannel.h

@@ -18,27 +18,77 @@ namespace Passenger {
 
 using namespace std;
 
+/**
+ * This class provides convenience methods for:
+ *  - sending and receiving discrete messages over a file descriptor.
+ *    A message is just a list of strings.
+ *  - file descriptor passing over a Unix socket.
+ *
+ * MessageChannel is to be wrapped around a file descriptor. For example:
+ * @code
+ *    int p[2];
+ *    pipe(p);
+ *    MessageChannel channel1(p[0]);
+ *    MessageChannel channel2(p[1]);
+ *    
+ *    channel2.write("hello", "world !!", NULL);
+ *    list<string> args;
+ *    channel1.read(args);  // args now contains { "hello", "world !!" }
+ * @endcode
+ *
+ * The life time of a MessageChannel is independent from that of the
+ * wrapped file descriptor. If a MessageChannel object is destroyed,
+ * the file descriptor is not automatically closed. Call close()
+ * if you want to close the file descriptor.
+ *
+ * @note I/O operations are not buffered.
+ * @note Be careful with mixing the sending/receiving of messages file
+ *       descriptor passing. These operations have stream properties.
+ *       Suppose you first send a message, then pass a file descriptor.
+ *       If the other side of the communication channel first tries to
+ *       receive a file descriptor, and then tries to receive a message,
+ *       then bad things will happen.
+ */
 class MessageChannel {
 private:
 	const static char DELIMITER = '\0';
 	int fd;
 
 public:
+	/**
+	 * Construct a new MessageChannel with no underlying file descriptor.
+	 * Thus the resulting MessageChannel object will not be usable.
+	 * This constructor exists to allow one to declare an "empty"
+	 * MessageChannel variable which is to be initialized later.
+	 */
 	MessageChannel() {
 		this->fd = -1;
 	}
 
+	/**
+	 * Construct a new MessageChannel with the given file descriptor.
+	 */
 	MessageChannel(int fd) {
 		this->fd = fd;
 	}
 
+	/**
+	 * Close the underlying file descriptor. If this method is called multiple
+	 * times, the file descriptor will only be closed the first time.
+	 */
 	void close() {
 		if (fd != -1) {
 			::close(fd);
 			fd = -1;
 		}
 	}
 
+	/**
+	 * Send the message, which consists of the given elements, over the underlying
+	 * file descriptor.
+	 *
+	 * @throws SystemException An error occured while writing the data to the file descriptor.
+	 */
 	void write(const list<string> &args) {
 		list<string>::const_iterator it;
 		string data;
@@ -70,6 +120,15 @@ class MessageChannel {
 		} while (written < data.size());
 	}
 
+	/**
+	 * Send the message, which consists of the given strings, over the underlying
+	 * file descriptor.
+	 *
+	 * @param name The first element of the message to send.
+	 * @param ... Other elements of the message. These *must* be strings, i.e. of type char*.
+	 *            It is also required to terminate this list with a NULL.
+	 * @throws SystemException An error occured while writing the data to the file descriptor.
+	 */
 	void write(const char *name, ...) {
 		list<string> args;
 		args.push_back(name);
@@ -88,6 +147,14 @@ class MessageChannel {
 		write(args);
 	}
 
+	/**
+	 * Pass a file descriptor. This only works if the underlying file
+	 * descriptor is a Unix socket.
+	 *
+	 * @param fileDescriptor The file descriptor to pass.
+	 * @throws SystemException Something went wrong during file descriptor passing.
+	 * @pre <tt>fileDescriptor >= 0</tt>
+	 */
 	void writeFileDescriptor(int fileDescriptor) {
 		struct msghdr msg;
 		struct iovec vec[1];
@@ -120,6 +187,14 @@ class MessageChannel {
 		}
 	}
 
+	/**
+	 * Receive a message from the underlying file descriptor.
+	 *
+	 * @param args The message will be put in this variable.
+	 * @return Whether end-of-file has been reached. If so, then the contents
+	 *         of <tt>args</tt> will be undefined.
+	 * @throws SystemException If an error occured while receiving the message.
+	 */
 	bool read(vector<string> &args) {
 		uint16_t size;
 		int ret;
@@ -165,6 +240,17 @@ class MessageChannel {
 		return true;
 	}
 
+	/**
+	 * Receive a file descriptor, which had been passed over the underlying
+	 * file descriptor.
+	 *
+	 * @return The passed file descriptor.
+	 * @throws SystemException If something went wrong during the
+	 *            receiving of a file descriptor. Perhaps the underlying
+	 *            file descriptor isn't a Unix socket.
+	 * @throws IOException Whatever was received doesn't seem to be a
+	 *            file descriptor.
+	 */
 	int readFileDescriptor() {
 		struct msghdr msg;
 		struct iovec vec[2];

ext/apache2/SpawnManager.h

@@ -23,29 +23,61 @@ using namespace std;
 using namespace boost;
 
 /**
- * This class is responsible for spawning Ruby on Rails applications.
- * TODO: write better documentation
+ * This class is responsible for spawning new instances of Ruby on Rails applications.
+ * Use the spawn() method to do so.
+ *
+ * <h2>Implementation details</h2>
+ * Internally, it makes use of a spawn server, which is written in Ruby. This server
+ * is automatically started when a SpawnManager instance is created, and automatically
+ * shutdown when that instance is destroyed. Spawning requests are sent to the server,
+ * and details about the spawned process is returned.
+ *
+ * The communication channel with the server is anonymous, i.e. no other processes
+ * can access the communication channel, so communication is guaranteed to be safe
+ * (unless, of course, if the spawn server itself is a trojan).
+ *
+ * The server will try to keep the spawning time as small as possible, by keeping
+ * corresponding Ruby on Rails frameworks and application code in memory. So the second
+ * time an instance of the same application is spawned, the spawn time is significantly
+ * lower than the first time. Nevertheless, spawning is a relatively expensive operation
+ * (compared to the processing of a typical HTTP request/response), and so should be
+ * avoided whenever possible.
+ *
+ * See the documentation of the spawn server for full implementation details.
  */
 class SpawnManager {
 private:
 	MessageChannel channel;
 	pid_t pid;
 
 public:
-	SpawnManager(const string &spawnManagerCommand,
+	/**
+	 * Construct a new SpawnManager.
+	 *
+	 * @param spawnServerCommand The filename of the spawn server to use.
+	 * @param logFile Specify a log file that the spawn manager should use.
+	 *            Messages on its standard output and standard error channels
+	 *            will be written to this log file. If an empty string is
+	 *            specified, no log file will be used, and the spawn server
+	 *            will use the same standard output/error channels as the
+	 *            current process.
+	 * @param environment The RAILS_ENV environment that all RoR applications
+	 *            should use. If an empty string is specified, the current value
+	 *            of the RAILS_ENV environment variable will be used.
+	 * @param rubyCommand The Ruby interpreter's command.
+	 * @param SystemException An error occured while trying to setup the spawn server.
+	 * @throws IOException The specified log file could not be opened.
+	 */
+	SpawnManager(const string &spawnServerCommand,
 	             const string &logFile = "",
 	             const string &environment = "production",
 	             const string &rubyCommand = "ruby") {
 		int fds[2];
-		char fd_string[20];
 		FILE *logFileHandle = NULL;
 
 		if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == -1) {
 			throw SystemException("Cannot create a Unix socket", errno);
 		}
-		if (apr_snprintf(fd_string, sizeof(fd_string), "%d", fds[1]) <= 0) {
-			throw MemoryException();
-		}
 		if (!logFile.empty()) {
 			logFileHandle = fopen(logFile.c_str(), "a");
 			if (logFileHandle == NULL) {
@@ -58,19 +90,28 @@ class SpawnManager {
 
 		pid = fork();
 		if (pid == 0) {
-			// TODO: redirect stderr to a log file
 			pid = fork();
 			if (pid == 0) {
 				if (!logFile.empty()) {
-					dup2(fileno(logFileHandle), STDOUT_FILENO);
 					dup2(fileno(logFileHandle), STDERR_FILENO);
 					fclose(logFileHandle);
 				}
+				dup2(STDERR_FILENO, STDOUT_FILENO);
 				if (!environment.empty()) {
 					setenv("RAILS_ENV", environment.c_str(), true);
 				}
+				dup2(fds[1], STDIN_FILENO);
 				close(fds[0]);
-				execlp(rubyCommand.c_str(), rubyCommand.c_str(), spawnManagerCommand.c_str(), fd_string, NULL);
+				close(fds[1]);
+
+				// Close all other file descriptors
+				for (int i = sysconf(_SC_OPEN_MAX); i >= 0; i--) {
+					if (i != STDIN_FILENO && i != STDOUT_FILENO && i != STDERR_FILENO) {
+						close(i);
+					}
+				}
+
+				execlp(rubyCommand.c_str(), rubyCommand.c_str(), spawnServerCommand.c_str(), NULL);
 				fprintf(stderr, "Unable to run %s: %s\n", rubyCommand.c_str(), strerror(errno));
 				_exit(1);
 			} else if (pid == -1) {
@@ -82,6 +123,9 @@ class SpawnManager {
 		} else if (pid == -1) {
 			close(fds[0]);
 			close(fds[1]);
+			if (logFileHandle != NULL) {
+				fclose(logFileHandle);
+			}
 			throw SystemException("Unable to fork a process", errno);
 		} else {
 			close(fds[1]);
@@ -93,7 +137,7 @@ class SpawnManager {
 		}
 	}
 
-	~SpawnManager() {
+	~SpawnManager() throw() {
 		channel.close();
 	}
 

lib/mod_rails/spawn_manager.rb

@@ -162,9 +162,8 @@ def reload
 end
 
 if __FILE__ == $0
-	unix_socket = IO.new(ARGV[0].to_i, "a+")
 	spawn_manager = SpawnManager.new
-	spawn_manager.server_main(unix_socket)
+	spawn_manager.server_main(IO.new(0, "a+"))
 	spawn_manager.cleanup
 end