Skip to content

Commit

Permalink
fix: Don’t make detached threads, and improve signal handling (#93).
Browse files Browse the repository at this point in the history
  • Loading branch information
Benjamin Geer committed Jan 5, 2017
1 parent 938bd15 commit 4365500
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 38 deletions.
72 changes: 61 additions & 11 deletions shttps/Server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ static std::mutex debugio; // mutex to protect debugging messages from threads

static std::vector<pthread_t> idle_thread_ids;

// The signal caught by the sig_thread function, used only for debugging.
static int signal_result = 0;

namespace shttps {

const char loggername[] = "Sipi"; // see Global.h !!
Expand All @@ -81,6 +84,39 @@ namespace shttps {
} TData;
//=========================================================================

/*!
* Starts a thread just to catch all signals sent to the server process.
* If it receives SIGINT, tells the server to stop.
*/
static void *sig_thread(void *arg) {
Server *serverptr = (Server *) arg;
sigset_t set;
sigemptyset(&set);
sigaddset(&set, SIGPIPE);
sigaddset(&set, SIGINT);

int s, sig;

for (;;) {
if ((s = sigwait(&set, &sig)) != 0) {
signal_result = -1;
return NULL;
}

signal_result = sig;

if (sig == SIGINT) {
serverptr->stop();
return NULL;
}
else {
signal_result = -1;
return NULL;
}
}
}
//=========================================================================


static void default_handler(Connection &conn, LuaServer &lua, void *user_data, void *hd)
{
Expand Down Expand Up @@ -638,8 +674,6 @@ namespace shttps {

static void *process_request(void *arg)
{
signal(SIGPIPE, SIG_IGN);

TData *tdata = (TData *) arg;
pthread_t my_tid = pthread_self();

Expand Down Expand Up @@ -774,6 +808,7 @@ namespace shttps {
if (close(tdata->commpipe_read) == -1) {
syslog(LOG_ERR, "Commpipe_write close error at [%s: %d]: %m", __file__, __LINE__);
}

int compipe_write = tdata->serv->get_thread_pipe(pthread_self());
if (compipe_write > 0) {
if (close (compipe_write) == -1) {
Expand All @@ -783,7 +818,9 @@ namespace shttps {
else {
syslog(LOG_DEBUG, "Thread to stop does not exist");
}

tdata->serv->remove_thread(pthread_self());

tdata->serv->semaphore_leave();

delete tdata;
Expand All @@ -795,6 +832,19 @@ namespace shttps {

void Server::run()
{
// Start a thread just to catch signals sent to the server process.
pthread_t sighandler_thread;
sigset_t set;
sigemptyset(&set);
sigaddset(&set, SIGINT);
sigaddset(&set, SIGPIPE);

int res;
if ((res = pthread_sigmask(SIG_BLOCK, &set, NULL)) != 0) {
syslog(LOG_ERR, "pthread_sigmask failed! (err=%d)", res);
}
res = pthread_create(&sighandler_thread, NULL, &sig_thread, (void *) this);

int old_ll = setlogmask(LOG_MASK(LOG_INFO));
syslog(LOG_INFO, "Starting shttps server with %d threads", _nthreads);
setlogmask(old_ll);
Expand Down Expand Up @@ -988,16 +1038,10 @@ namespace shttps {

tmp->commpipe_read = commpipe[1]; // read end;

//
// we create detached threads because we will not be able to use pthread_join()
//
pthread_attr_t tattr;
pthread_attr_init(&tattr);
pthread_attr_setdetachstate(&tattr, PTHREAD_CREATE_DETACHED);
//int stacksize = (PTHREAD_STACK_MIN + 1024*1024*3);
//pthread_attr_setstacksize(&tattr, stacksize);

if( pthread_create( &thread_id, &tattr, process_request, (void *) tmp) < 0) {
if (pthread_create(&thread_id, &tattr, process_request, (void *) tmp) < 0) {
syslog(LOG_ERR, "Could not create thread at [%s: %d]: %m", __file__, __LINE__);
running = false;
break;
Expand All @@ -1017,6 +1061,7 @@ namespace shttps {
syslog(LOG_INFO, "Server shutting down");
setlogmask(old_ll);


//
// let's send the close message to all running threads
//
Expand All @@ -1037,11 +1082,16 @@ namespace shttps {
// we have closed all sockets, now we can wait for the threads to terminate
//
for (int i = 0; i < num_active_threads; i++) {
pthread_join(ptid[i], NULL);
int err;
if ((err = pthread_join(ptid[i], NULL)) != 0) {
syslog(LOG_ERR, "pthread_join failed with error code %d", err);
}
}

delete [] ptid;
}

// std::cerr << "signal_result is " << signal_result << std::endl;
}
//=========================================================================


Expand Down
29 changes: 2 additions & 27 deletions src/sipi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <fstream>
#include <sstream>
#include <thread>
#include <csignal>
#include <utility>
#include <stdlib.h>

Expand Down Expand Up @@ -78,9 +77,6 @@ Sipi::SipiHttpServer *serverptr = NULL;
Sipi::SipiConf sipiConf;
enum FileType {image, video, audio, text, binary};

static sig_t old_sighandler;
static sig_t old_broken_pipe_handler;

static std::string fileType_string(FileType f_type) {
std::string type_string;

Expand All @@ -105,25 +101,6 @@ static std::string fileType_string(FileType f_type) {
return type_string;
};

static void sighandler(int sig) {
// Any functions called in a signal handler must be asynchronous-safe.
// See https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers

if (serverptr != NULL) {
// Server::stop() is asynchronous-safe.
serverptr->stop();
}
}
//=========================================================================


static void broken_pipe_handler(int sig) {
syslog(LOG_INFO, "Got BROKEN PIPE signal!");
}
//=========================================================================




static void send_error(shttps::Connection &conobj, shttps::Connection::StatusCodes code, std::string msg)
{
Expand Down Expand Up @@ -265,6 +242,7 @@ int main (int argc, char *argv[]) {
}
} sipiInit;


//
// commandline processing....
//
Expand Down Expand Up @@ -415,9 +393,8 @@ int main (int argc, char *argv[]) {
}

serverptr = &server;
old_sighandler = signal(SIGINT, sighandler);
old_broken_pipe_handler = signal(SIGPIPE, broken_pipe_handler);
server.run();

}
catch (shttps::Error &err) {
std::cerr << err << std::endl;
Expand All @@ -433,8 +410,6 @@ int main (int argc, char *argv[]) {
Sipi::SipiHttpServer server((params["serverport"])[0].getValue (SipiIntType), nthreads);
server.imgroot((params["imgroot"])[0].getValue(SipiStringType));
serverptr = &server;
old_sighandler = signal(SIGINT, sighandler);
old_broken_pipe_handler = signal(SIGPIPE, broken_pipe_handler);
server.run();
}
else {
Expand Down

0 comments on commit 4365500

Please sign in to comment.