Skip to content

Commit

Permalink
Fix shutdown issue with stdin/ctrl socket once and for all
Browse files Browse the repository at this point in the history 
Also fix so stdin is usable in your terminal after SIPp ends.

Fixes #354 and #359. Thanks Jeannot Langlois for pointing out the crashes and
testing these fixes.
  • Loading branch information
@wdoekes
wdoekes committed Jul 13, 2018
1 parent b0d56e1 commit 5c15e77
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 37 deletions.
4 changes: 2 additions & 2 deletions regress/github-#0156/run
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ which valgrind >/dev/null || fail "no valgrind installed"

# Valgrind on UAS.
valgrind --xml=yes --xml-file=uas.log --partial-loads-ok=yes \
--show-leak-kinds=all --leak-check=full `get_sipp` -nostdin -m 10 -sn uas \
--show-leak-kinds=all --leak-check=full `get_sipp` -m 10 -sn uas \
>/dev/null 2>&1 &
uaspid=$!
sleep 1

# Valgrind on UAC.
valgrind --xml=yes --xml-file=uac.log --partial-loads-ok=yes \
--show-leak-kinds=all --leak-check=full `get_sipp` -nostdin -m 10 -sn uac \
--show-leak-kinds=all --leak-check=full `get_sipp` -m 10 -sn uac \
127.0.0.1 >/dev/null 2>&1 &
uacpid=$!

Expand Down
42 changes: 18 additions & 24 deletions src/sipp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ void timeout_alarm(int /*param*/)

/* Send loop & trafic generation*/

static bool traffic_thread()
static void traffic_thread()
{
/* create the file */
char L_file_name[MAX_PATH];
Expand Down Expand Up @@ -493,23 +493,30 @@ static bool traffic_thread()
/* Force exit: abort all calls */
abort_all_tasks();
}
/* Quitting and no more openned calls, close all */
/* Quitting and no more opened calls, close all */
if (!main_scenario->stats->GetStat(CStat::CPT_C_CurrentCall)) {
/* We can have calls that do not count towards our open-call count (e.g., dead calls). */
abort_all_tasks();
#ifdef RTP_STREAM
rtpstream_shutdown();
#endif
for (unsigned i = 0; i < pollnfds; i++) {
/* Reverse order shutdown, because deleting reorders the
* sockets list. */
for (int i = pollnfds - 1; i >= 0; --i) {
sockets[i]->close();
if (sockets[i] == ctrl_socket) {
ctrl_socket = NULL;
} else if (sockets[i] == stdin_socket) {
stdin_socket = NULL;
}
}

screentask::report(true);
stattask::report();
if (useScreenf == 1) {
print_screens();
}
return false;
return;
}
}

Expand Down Expand Up @@ -545,9 +552,9 @@ static bool traffic_thread()
task * last = NULL;

task_list::iterator iter;
for(iter = running_tasks->begin(); iter != running_tasks->end(); iter++) {
for (iter = running_tasks->begin(); iter != running_tasks->end(); iter++) {
if (last) {
last -> run();
last->run();
if (sockets_pending_reset.begin() != sockets_pending_reset.end()) {
last = NULL;
break;
Expand All @@ -559,7 +566,7 @@ static bool traffic_thread()
}
}
if (last) {
last -> run();
last->run();
}
while (sockets_pending_reset.begin() != sockets_pending_reset.end()) {
(*(sockets_pending_reset.begin()))->reset_connection();
Expand All @@ -571,7 +578,7 @@ static bool traffic_thread()
/* Receive incoming messages */
SIPpSocket::pollset_process(running_tasks->empty());
}
return true;
assert(0);
}

/*************** RTP ECHO THREAD ***********************/
Expand Down Expand Up @@ -1070,7 +1077,7 @@ static void set_scenario(const char* name)
static int bind_rtp_sockets(struct sockaddr_storage* media_sa, int try_port, int last_attempt)
{
/* Create RTP sockets for audio and video. */
if ((media_socket_audio = socket(media_sa->ss_family, SOCK_DGRAM, 0)) == -1) {
if ((media_socket_audio = socket(media_sa->ss_family, SOCK_DGRAM, IPPROTO_UDP)) == -1) {
ERROR_NO("Unable to create the audio RTP socket");
}

Expand All @@ -1091,7 +1098,7 @@ static int bind_rtp_sockets(struct sockaddr_storage* media_sa, int try_port, int

/* Create and bind the second/video socket to try_port+2 */
/* (+1 is reserved for RTCP) */
if ((media_socket_video = socket(media_sa->ss_family, SOCK_DGRAM, 0)) == -1) {
if ((media_socket_video = socket(media_sa->ss_family, SOCK_DGRAM, IPPROTO_UDP)) == -1) {
ERROR_NO("Unable to create the video RTP socket");
}

Expand Down Expand Up @@ -2000,15 +2007,7 @@ int main(int argc, char *argv[])
}
}

if (traffic_thread()) {
/* Clean up */
if (!nostdin) {
stdin_socket->close();
stdin_socket = NULL; /* close "delete's this" */
}
ctrl_socket->close();
ctrl_socket = NULL; /* close "delete's this" */
}
traffic_thread();

/* Cancel and join other threads. */
if (pthread2_id) {
Expand All @@ -2024,11 +2023,6 @@ int main(int argc, char *argv[])
pthread_join(pthread3_id, NULL);
}

if (ctrl_socket != NULL) {
ctrl_socket->close(); /* also uses epoll */
ctrl_socket = NULL; /* close "delete's this" */
}

#ifdef HAVE_EPOLL
close(epollfd);
free(epollevents);
Expand Down
40 changes: 29 additions & 11 deletions src/socket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ extern bool show_index;
SIPpSocket *ctrl_socket = NULL;
SIPpSocket *stdin_socket = NULL;

static int stdin_fileno = -1;
static int stdin_mode;

/******************** Recv Poll Processing *********************/

unsigned pollnfds;
Expand Down Expand Up @@ -485,7 +488,7 @@ void setup_ctrl_socket()
int try_counter = 60;
struct sockaddr_storage ctl_sa;

int sock = socket(AF_INET, SOCK_DGRAM, 0);
int sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (sock == -1) {
ERROR_NO("Unable to create remote control socket!");
}
Expand Down Expand Up @@ -541,11 +544,19 @@ void setup_ctrl_socket()
}
}

void reset_stdin()
{
fcntl(stdin_fileno, F_SETFL, stdin_mode);
}

void setup_stdin_socket()
{
int stdin_mode = fcntl(fileno(stdin), F_GETFL);
fcntl(fileno(stdin), F_SETFL, stdin_mode | O_NONBLOCK);
stdin_socket = new SIPpSocket(0, T_UDP, fileno(stdin), 0);
stdin_fileno = fileno(stdin);
stdin_mode = fcntl(stdin_fileno, F_GETFL);
atexit(reset_stdin);
fcntl(stdin_fileno, F_SETFL, stdin_mode | O_NONBLOCK);

stdin_socket = new SIPpSocket(0, T_TCP, stdin_fileno, 0);
if (!stdin_socket) {
ERROR_NO("Could not setup keyboard (stdin) socket!\n");
}
Expand Down Expand Up @@ -946,12 +957,12 @@ void SIPpSocket::invalidate()
}
#endif

if (::close(ss_fd) < 0) {
if (ss_fd == stdin_fileno) {
/* don't close stdin, breaks interactive terminals */
} else if (::close(ss_fd) < 0) {
WARNING_NO("Failed to close socket %d", ss_fd);
}
else {
ss_fd = -1;
}
ss_fd = -1;
}

if ((pollidx = ss_pollidx) >= pollnfds) {
Expand Down Expand Up @@ -982,8 +993,11 @@ void SIPpSocket::invalidate()
#else
pollfiles[pollidx] = pollfiles[pollnfds];
#endif
sockets[pollidx] = sockets[pollnfds];
sockets[pollidx]->ss_pollidx = pollidx;
/* If unequal, move the last valid socket here. */
if (pollidx != pollnfds) {
sockets[pollidx] = sockets[pollnfds];
sockets[pollidx]->ss_pollidx = pollidx;
}
sockets[pollnfds] = NULL;

if (ss_msglen) {
Expand Down Expand Up @@ -1314,6 +1328,7 @@ static int socket_fd(bool use_ipv6, int transport)
#endif
case T_TCP:
socket_type = SOCK_STREAM;
protocol = IPPROTO_TCP;
break;
}

Expand Down Expand Up @@ -2391,7 +2406,9 @@ int open_connections()
::connect(tmpsock, _RCAST(struct sockaddr*, &remote_sockaddr),
socklen_from_addr(&remote_sockaddr)) < 0 ||
getsockname(tmpsock, _RCAST(struct sockaddr*, &local_sockaddr), &len) < 0) {
// close(tmpsock);
if (tmpsock >= 0) {
close(tmpsock);
}
ERROR_NO("Failed to find our local ip");
}
close(tmpsock);
Expand Down Expand Up @@ -2552,6 +2569,7 @@ int open_connections()
if (reset_number > 0) {
WARNING("Failed to reconnect\n");
main_socket->close();
main_socket = NULL;
reset_number--;
return 1;
} else {
Expand Down

0 comments on commit 5c15e77

Please sign in to comment.