r39456 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r39455‎ | r39456 | r39457 >
Date:04:39, 16 August 2008
Author:river
Status:old
Tags:
Comment:
split acceptor_thread into its own class
add log-request-errors config option
Modified paths:
  • /trunk/tools/switchboard2/Makefile (modified) (history)
  • /trunk/tools/switchboard2/acceptor_thread.cc (added) (history)
  • /trunk/tools/switchboard2/acceptor_thread.h (added) (history)
  • /trunk/tools/switchboard2/config.cc (modified) (history)
  • /trunk/tools/switchboard2/config.h (modified) (history)
  • /trunk/tools/switchboard2/main.cc (modified) (history)
  • /trunk/tools/switchboard2/request_thread.cc (modified) (history)
  • /trunk/tools/switchboard2/request_thread.h (modified) (history)
  • /trunk/tools/switchboard2/version.h (modified) (history)

Diff [purge]

Index: trunk/tools/switchboard2/config.cc
——@@ -31,6 +31,7 @@
3232 , php_timeout(30)
3333 , server_timeout(30)
3434 , max_request_size(1024 * 1024 * 5)
 35+ , log_request_errors(false)
3536 {
3637 }
3738
——@@ -75,18 +76,19 @@
7677 std::map<std::string, configuration_loader::confline_t>
7778 configuration_loader::conflines =
7879 boost::assign::map_list_of
79 - ("listen", boost::bind(&configuration_loader::f_listen, _1, _2, _3))
 80+ ("listen", configuration_loader::confline_t(boost::bind(&configuration_loader::f_listen, _1, _2, _3)))
8081 ("logconf", boost::bind(&configuration_loader::f_logconf, _1, _2, _3))
8182 ("sockdir", boost::bind(&configuration_loader::f_sockdir, _1, _2, _3))
8283 ("docroot", boost::bind(&configuration_loader::f_docroot, _1, _2, _3))
8384 ("userdir", boost::bind(&configuration_loader::f_userdir, _1, _2, _3))
 85+ ("server-type", boost::bind(&configuration_loader::f_server_type, _1, _2, _3))
8486 ("max-procs", boost::bind(&configuration_loader::f_max_procs, _1, _2, _3))
8587 ("max-procs-per-user", boost::bind(&configuration_loader::f_max_procs_per_user, _1, _2, _3))
8688 ("max-q-per-user", boost::bind(&configuration_loader::f_max_q_per_user, _1, _2, _3))
87 - ("server-type", boost::bind(&configuration_loader::f_server_type, _1, _2, _3))
8889 ("php-timeout", boost::bind(&configuration_loader::f_php_timeout, _1, _2, _3))
8990 ("server-timeout", boost::bind(&configuration_loader::f_server_timeout, _1, _2, _3))
9091 ("max-request-size", boost::bind(&configuration_loader::f_max_request_size, _1, _2, _3))
 92+ ("log-request-errors", boost::bind(&configuration_loader::f_log_request_errors, _1, _2, _3))
9193 ;
9294
9395 bool
——@@ -370,3 +372,19 @@
371373 return false;
372374 }
373375 }
 376+
 377+bool
 378+configuration_loader::f_log_request_errors(
 379+ std::vector<std::string> &fields,
 380+ config &newconf)
 381+{
 382+ if (fields.size() != 1) {
 383+ LOG4CXX_ERROR(logger,
 384+ format("\"%s\", line %d: usage: log-request-errors\n")
 385+ % file_ % lineno_);
 386+ return false;
 387+ }
 388+
 389+ newconf.log_request_errors = true;
 390+ return true;
 391+}
Index: trunk/tools/switchboard2/config.h
——@@ -48,8 +48,9 @@
4949 int max_q_per_user;
5050 int php_timeout;
5151 int server_timeout;
52 - std::size_t max_request_size;
 52+ int max_request_size;
5353 server_type servtype;
 54+ bool log_request_errors;
5455 };
5556
5657 struct configuration_loader {
——@@ -81,6 +82,7 @@
8283 bool f_server_timeout(std::vector<std::string> &fields, config &newconf);
8384 bool f_php_timeout(std::vector<std::string> &fields, config &newconf);
8485 bool f_max_request_size(std::vector<std::string> &fields, config &newconf);
 86+ bool f_log_request_errors(std::vector<std::string> &fields, config &newconf);
8587
8688 log4cxx::LoggerPtr logger;
8789 };
Index: trunk/tools/switchboard2/request_thread.cc
——@@ -32,7 +32,12 @@
3333 request_thread *req = static_cast<request_thread *>(arg);
3434 try {
3535 req->start_request();
36 - } catch (std::exception &) {
 36+ } catch (std::exception &e) {
 37+ if (mainconf.log_request_errors) {
 38+ log4cxx::LoggerPtr logger(log4cxx::Logger::getLogger("request_thread"));
 39+ LOG4CXX_INFO(logger,
 40+ boost::format("request failed: %s") % e.what());
 41+ }
3742 }
3843
3944 delete req;
——@@ -61,10 +66,10 @@
6267 void
6368 request_thread::start()
6469 {
65 - pthread_attr_t attr;
66 - pthread_attr_init(&attr);
67 - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
68 - pthread_create(&tid_, &attr, &do_start_thread, static_cast<void *>(this));
 70+ pthread_attr_t attr;
 71+ pthread_attr_init(&attr);
 72+ pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 73+ pthread_create(&tid_, &attr, &do_start_thread, static_cast<void *>(this));
6974 }
7075
7176 void
——@@ -88,6 +93,12 @@
8994 try {
9095 handle_normal_request(rec);
9196 } catch (std::exception &e) {
 97+ if (mainconf.log_request_errors) {
 98+ log4cxx::LoggerPtr logger(log4cxx::Logger::getLogger("request_thread"));
 99+ LOG4CXX_INFO(logger,
 100+ boost::format("request failed: %s") % e.what());
 101+ }
 102+
92103 fcgi::record r;
93104
94105 r.version_ = 1;
Index: trunk/tools/switchboard2/request_thread.h
——@@ -16,6 +16,8 @@
1717
1818 #include <pthread.h>
1919
 20+#include <boost/format.hpp>
 21+
2022 #include "fcgi.h"
2123 #include "process.h"
2224
Index: trunk/tools/switchboard2/main.cc
——@@ -41,9 +41,8 @@
4242 #include "config.h"
4343 #include "version.h"
4444 #include "process_factory.h"
 45+#include "acceptor_thread.h"
4546
46 - extern "C" void * acceptor_thread(void *);
47 -
4847 extern "C" void
4948 sigexit(int)
5049 {
——@@ -148,8 +147,8 @@
149148 return 1;
150149 }
151150
152 - pthread_t tid;
153 - pthread_create(&tid, NULL, acceptor_thread, reinterpret_cast<void *>((intptr_t) lsnsock));
 151+ acceptor_thread *acc = new acceptor_thread(lsnsock);
 152+ acc->run();
154153 } else {
155154 int r;
156155 struct addrinfo hints, *res, *iter;
——@@ -201,8 +200,8 @@
202201 return 1;
203202 }
204203
205 - pthread_t tid;
206 - pthread_create(&tid, NULL, acceptor_thread, reinterpret_cast<void *>((intptr_t) lsnsock));
 204+ acceptor_thread *acc = new acceptor_thread(lsnsock);
 205+ acc->run();
207206 }
208207
209208 freeaddrinfo(res);
——@@ -352,20 +351,3 @@
353352
354353 process_factory::instance().cleanup_thread();
355354 }
356 -
357 -extern "C" void *
358 -acceptor_thread(void *arg)
359 -{
360 - int fd = reinterpret_cast<intptr_t>(arg);
361 - int newfd;
362 - struct sockaddr addr;
363 - socklen_t addrlen = sizeof(addr);
364 -
365 - while ((newfd = accept(fd, &addr, &addrlen)) != -1) {
366 - request_thread *req = new request_thread(newfd);
367 - req->start();
368 - }
369 -
370 - std::printf("accept failed: %s\n", std::strerror(errno));
371 - return NULL;
372 -}
Index: trunk/tools/switchboard2/acceptor_thread.cc
——@@ -0,0 +1,59 @@
 2+/* Copyright (c) 2008 River Tarnell <river@wikimedia.org>. */
 3+/*
 4+ * Permission is granted to anyone to use this software for any purpose,
 5+ * including commercial applications, and to alter it and redistribute it
 6+ * freely. This software is provided 'as-is', without any express or implied
 7+ * warranty.
 8+ */
 9+/* $Id$ */
 10+
 11+#include <sys/types.h>
 12+#include <sys/socket.h>
 13+
 14+#include "acceptor_thread.h"
 15+#include "request_thread.h"
 16+
 17+namespace {
 18+
 19+extern "C" void *
 20+start_acceptor(void *arg)
 21+{
 22+ acceptor_thread *thr = static_cast<acceptor_thread *>(arg);
 23+ thr->start();
 24+ return NULL;
 25+}
 26+
 27+} // anonymous namespace
 28+
 29+acceptor_thread::acceptor_thread(int fd)
 30+ : fd_(fd)
 31+ , logger(log4cxx::Logger::getLogger("acceptor"))
 32+{
 33+}
 34+
 35+acceptor_thread::~acceptor_thread()
 36+{
 37+ close(fd_);
 38+}
 39+
 40+void
 41+acceptor_thread::run()
 42+{
 43+ pthread_t tid;
 44+ pthread_create(&tid, NULL, start_acceptor, this);
 45+}
 46+
 47+void
 48+acceptor_thread::start()
 49+{
 50+ int newfd;
 51+ struct sockaddr addr;
 52+ socklen_t addrlen = sizeof(addr);
 53+
 54+ while ((newfd = accept(fd_, &addr, &addrlen)) != -1) {
 55+ request_thread *req = new request_thread(newfd);
 56+ req->start();
 57+ }
 58+
 59+ LOG4CXX_ERROR(logger, boost::format("accept failed: %s") % std::strerror(errno));
 60+}
Index: trunk/tools/switchboard2/version.h
——@@ -10,6 +10,6 @@
1111 #ifndef VERSION_H
1212 #define VERSION_H
1313
14 -#define SB_VERSION "V-2.0.8"
 14+#define SB_VERSION "V-2.0.9"
1515
1616 #endif /* !VERSION_H */
Index: trunk/tools/switchboard2/acceptor_thread.h
——@@ -0,0 +1,27 @@
 2+/* Copyright (c) 2008 River Tarnell <river@wikimedia.org>. */
 3+/*
 4+ * Permission is granted to anyone to use this software for any purpose,
 5+ * including commercial applications, and to alter it and redistribute it
 6+ * freely. This software is provided 'as-is', without any express or implied
 7+ * warranty.
 8+ */
 9+/* $Id$ */
 10+
 11+#ifndef ACCEPTOR_THREAD_H
 12+#define ACCEPTOR_THREAD_H
 13+
 14+#include <log4cxx/logger.h>
 15+
 16+struct acceptor_thread {
 17+ acceptor_thread(int);
 18+ ~acceptor_thread();
 19+
 20+ void start();
 21+ void run();
 22+
 23+private:
 24+ int fd_;
 25+ log4cxx::LoggerPtr logger;
 26+};
 27+
 28+#endif /* !ACCEPTOR_THREAD_H */
Index: trunk/tools/switchboard2/Makefile
——@@ -40,6 +40,7 @@
4141 config.o \
4242 process_factory.o \
4343 util.o \
 44+ acceptor_thread.o \
4445
4546 PROG= switchboard-bin
4647 SWEXEC= swexec
——@@ -106,7 +107,7 @@
107108
108109 # Do not delete this line -- make depend requires it
109110 main.o: main.cc fcgi.h request_thread.h process.h config.h version.h \
110 - process_factory.h
 111+ process_factory.h acceptor_thread.h
111112 request_thread.o: request_thread.cc request_thread.h fcgi.h process.h \
112113 process_factory.h util.h config.h
113114 fcgi.o: fcgi.cc fcgi.h util.h
——@@ -115,5 +116,7 @@
116117 process_factory.o: process_factory.cc process_factory.h process.h \
117118 config.h util.h
118119 util.o: util.cc util.h
 120+acceptor_thread.o: acceptor_thread.cc acceptor_thread.h request_thread.h \
 121+ fcgi.h process.h
119122 swexec.o: swexec.c
120123 swkill.o: swkill.c

Status & tagging log