From 8b5c962e0bb039e7e7c7bc36c62882b19434b061 Mon Sep 17 00:00:00 2001 From: Walter Doekes Date: Sun, 12 Jul 2015 15:50:31 +0200 Subject: [PATCH] Fix a few memory leaks at shutdown. To run valgrind: # make sipp without optimization make CFLAGS="-g -O0" CXXFLAGS="-g -O0" # run sipp with valgrind valgrind --partial-loads-ok=yes --show-leak-kinds=all \ --leak-check=full \ sipp SIPP_ARGUMENTS | cat # (the extra cat disables curses, which can show distracting leaks) See also the new regression test in regress/github-#0156. --- include/screen.hpp | 2 +- include/socket.hpp | 2 - regress/github-#0156/run | 28 +++++++++ src/call.cpp | 23 ++++--- src/screen.cpp | 11 ++-- src/sipp.cpp | 129 +++++++++++++++++++++++---------------- src/socket.cpp | 53 ++++++---------- 7 files changed, 145 insertions(+), 103 deletions(-) create mode 100755 regress/github-#0156/run diff --git a/include/screen.hpp b/include/screen.hpp index 90e417c13..65ad2c834 100644 --- a/include/screen.hpp +++ b/include/screen.hpp @@ -49,7 +49,7 @@ extern "C" { #define EXIT_FATAL_ERROR -1 #define EXIT_BIND_ERROR -2 -void screen_set_exename(char * exe_name); +void screen_set_exename(const char* exe_name); void screen_init(); void screen_clear(); int screen_readkey(); diff --git a/include/socket.hpp b/include/socket.hpp index cc6de26c8..7ba4638c7 100644 --- a/include/socket.hpp +++ b/include/socket.hpp @@ -49,8 +49,6 @@ struct sipp_socket *sipp_allocate_socket(bool use_ipv6, int transport, int fd, i void setup_ctrl_socket(); void setup_stdin_socket(); -char * get_inet_address(struct sockaddr_storage * addr); - int handle_ctrl_socket(); void handle_stdin_socket(); void process_message(struct sipp_socket *socket, char *msg, ssize_t msg_size, struct sockaddr_storage *src); diff --git a/regress/github-#0156/run b/regress/github-#0156/run new file mode 100755 index 000000000..a73e5ea97 --- /dev/null +++ b/regress/github-#0156/run @@ -0,0 +1,28 @@ +#!/bin/sh +# This regression test is a part of SIPp. +# Author: Walter Doekes, OSSO B.V., 2015 +. "`dirname "$0"`/../functions"; init + +# 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 \ + >/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 \ + 127.0.0.1 >/dev/null 2>&1 & +uacpid=$! + +wait $uaspid +test $? -ne 0 && fail "UAS returned non-zero" + +wait $uacpid +test $? -ne 0 && fail "UAC returned non-zero" + +grep -q '' uas.log && fail "valgrind reported leaks in UAS" +grep -q '' uac.log && fail "valgrind reported leaks in UAC" + +ok diff --git a/src/call.cpp b/src/call.cpp index 4c526a330..d239d920e 100644 --- a/src/call.cpp +++ b/src/call.cpp @@ -915,16 +915,15 @@ bool call::connect_socket_if_needed() NULL, &hints, &h); - memcpy(&saddr, - h->ai_addr, - SOCK_ADDR_SIZE( - _RCAST(struct sockaddr_storage *,h->ai_addr))); + memcpy(&saddr, h->ai_addr, + SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage*, h->ai_addr))); if (use_ipv6) { - (_RCAST(struct sockaddr_in6 *, &saddr))->sin6_port = htons(local_port); + (_RCAST(struct sockaddr_in6*, &saddr))->sin6_port = htons(local_port); } else { - (_RCAST(struct sockaddr_in *, &saddr))->sin_port = htons(local_port); + (_RCAST(struct sockaddr_in*, &saddr))->sin_port = htons(local_port); } + freeaddrinfo(h); } if (sipp_bind_socket(call_socket, &saddr, &call_port)) { @@ -3661,13 +3660,17 @@ call::T_ActionResult call::executeAction(char * msg, message *curmsg) if (_RCAST(struct sockaddr_storage *, local_addr->ai_addr)->ss_family != call_peer.ss_family) { ERROR("Can not switch between IPv4 and IPV6 using setdest!"); } - memcpy(&call_peer, local_addr->ai_addr, SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage *,local_addr->ai_addr))); + memcpy(&call_peer, local_addr->ai_addr, + SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage*, local_addr->ai_addr))); + freeaddrinfo(local_addr); + if (call_peer.ss_family == AF_INET) { - (_RCAST(struct sockaddr_in *,&call_peer))->sin_port = htons(port); + (_RCAST(struct sockaddr_in*, &call_peer))->sin_port = htons(port); } else { - (_RCAST(struct sockaddr_in6 *,&call_peer))->sin6_port = htons(port); + (_RCAST(struct sockaddr_in6*, &call_peer))->sin6_port = htons(port); } - memcpy(&call_socket->ss_dest, &call_peer, SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage *,&call_peer))); + memcpy(&call_socket->ss_dest, &call_peer, + SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage*, &call_peer))); free(str_host); free(str_port); diff --git a/src/screen.cpp b/src/screen.cpp index cbdcdda2b..7dc0af7f2 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -35,7 +35,7 @@ #include #ifdef __SUNOS -#include +#include #endif #include @@ -67,8 +67,11 @@ int screen_readkey() void screen_exit() { - if( backgroundMode == false ) - endwin(); + if (!screen_inited) { + return; + } + + endwin(); } void screen_show_errors() { @@ -96,7 +99,7 @@ void screen_clear() printf("\033[2J"); } -void screen_set_exename(char * exe_name) +void screen_set_exename(const char* exe_name) { strcpy(screen_exename, exe_name); } diff --git a/src/sipp.cpp b/src/sipp.cpp index efd263fba..4e60be38d 100644 --- a/src/sipp.cpp +++ b/src/sipp.cpp @@ -748,8 +748,7 @@ static void traffic_thread() if (useScreenf == 1) { print_screens(); } - - sipp_exit(EXIT_TEST_RES_UNKNOWN); + break; } } @@ -818,7 +817,7 @@ static void traffic_thread() static void rtp_echo_thread(void* param) { - char msg[media_bufsize]; + char* msg = (char*)alloca(media_bufsize); size_t nr, ns; sipp_socklen_t len; struct sockaddr_storage remote_rtp_addr; @@ -838,7 +837,7 @@ static void rtp_echo_thread(void* param) nr = recvfrom(*(int *)param, msg, media_bufsize, 0, - (sockaddr *)(void *) &remote_rtp_addr, + (sockaddr*)&remote_rtp_addr, &len); if (((long)nr) < 0) { @@ -848,7 +847,7 @@ static void rtp_echo_thread(void* param) return; } ns = sendto(*(int *)param, msg, nr, - 0, (sockaddr *)(void *) &remote_rtp_addr, + 0, (sockaddr*)&remote_rtp_addr, len); if (ns != nr) { @@ -1196,6 +1195,7 @@ static void releaseGlobalAllocations() free_default_messages(); freeInFiles(); freeUserVarMap(); + delete userVariables; delete globalVariables; } @@ -1219,6 +1219,17 @@ void sipp_exit(int rc) print_last_stats(); screen_show_errors(); + /* Close open files. */ + struct logfile_info** logfile_ptr; + struct logfile_info* logfiles[] = { + &screen_lfi, &calldebug_lfi, &message_lfi, &shortmessage_lfi, &log_lfi, &error_lfi, NULL}; + for (logfile_ptr = logfiles; *logfile_ptr; ++logfile_ptr) { + if ((*logfile_ptr)->fptr) { + fclose((*logfile_ptr)->fptr); + (*logfile_ptr)->fptr = NULL; + } + } + // Get failed calls counter value before releasing objects if (display_scenario) { counter_value_failed = display_scenario->stats->GetStat(CStat::CPT_C_FailedCall); @@ -1292,7 +1303,7 @@ int main(int argc, char *argv[]) { int argi = 0; struct sockaddr_storage media_sockaddr; - pthread_t pthread2_id, pthread3_id; + pthread_t pthread2_id = 0, pthread3_id = 0; unsigned int generic_count = 0; bool slave_masterSet = false; @@ -1328,19 +1339,16 @@ int main(int argc, char *argv[]) sigaction(SIGUSR2, &action_usr2, NULL); } - screen_set_exename((char *)"sipp"); + screen_set_exename("sipp"); pid = getpid(); memset(local_ip, 0, 40); #ifdef USE_SCTP memset(multihome_ip, 0, 40); #endif - memset(media_ip,0, 40); - memset(control_ip,0, 40); - memset(media_ip_escaped,0, 42); - - /* Load compression pluggin if available */ - comp_load(); + memset(media_ip, 0, 40); + memset(control_ip, 0, 40); + memset(media_ip_escaped, 0, 42); /* Initialize the tolower table. */ init_tolower_table(); @@ -1547,7 +1555,7 @@ int main(int argc, char *argv[]) break; case 'c': if(strlen(comp_error)) { - ERROR("No " COMP_PLUGGIN " pluggin available:\n%s", comp_error); + ERROR("No " COMP_PLUGGIN " plugin available:\n%s", comp_error); } transport = T_UDP; compression = 1; @@ -1751,11 +1759,9 @@ int main(int argc, char *argv[]) _RCAST(struct sockaddr_storage *, local_addr->ai_addr))); if (remote_sending_sockaddr.ss_family == AF_INET) { - (_RCAST(struct sockaddr_in *, &remote_sending_sockaddr))->sin_port = - htons((short)remote_s_p); + (_RCAST(struct sockaddr_in*, &remote_sending_sockaddr))->sin_port = htons(remote_s_p); } else { - (_RCAST(struct sockaddr_in6 *, &remote_sending_sockaddr))->sin6_port = - htons((short)remote_s_p); + (_RCAST(struct sockaddr_in6*, &remote_sending_sockaddr))->sin6_port = htons(remote_s_p); } use_remote_sending_addr = 1; @@ -1858,22 +1864,24 @@ int main(int argc, char *argv[]) ((struct logfile_info*)option->data)->overwrite = get_bool(argv[argi], argv[argi-1]); break; case SIPP_OPTION_PLUGIN: { - void *handle; - char *error; - int (*init)(); int ret; REQUIRE_ARG(); CHECK_PASS(); - handle = dlopen(argv[argi], RTLD_NOW); + void* handle = dlopen(argv[argi], RTLD_NOW); if (!handle) { ERROR("Could not open plugin %s: %s", argv[argi], dlerror()); } - init = (int (*)())dlsym(handle, "init"); - if((error = (char *) dlerror())) { - ERROR("Could not locate init function in %s: %s", argv[argi], dlerror()); + int (*init)(); + void* funcptr = dlsym(handle, "init"); + /* http://stackoverflow.com/questions/1096341/function-pointers-casting-in-c */ + *reinterpret_cast(&init) = funcptr; // yuck + + const char* error; + if ((error = dlerror())) { + ERROR("Could not locate init function in %s: %s", argv[argi], error); } ret = init(); @@ -1888,6 +1896,11 @@ int main(int argc, char *argv[]) } } + /* Load compression plugin if needed/available. */ + if (compression) { + comp_load(); + } + if((extendedTwinSippMode && !slave_masterSet) || (!extendedTwinSippMode && slave_masterSet)) { ERROR("-slave_cfg option must be used with -slave or -master option\n"); } @@ -2158,16 +2171,12 @@ int main(int argc, char *argv[]) if((media_socket = socket(media_ip_is_ipv6 ? AF_INET6 : AF_INET, SOCK_DGRAM, 0)) == -1) { - char msg[512]; - sprintf(msg, "Unable to get the audio RTP socket (IP=%s, port=%d)", media_ip, media_port); - ERROR_NO(msg); + ERROR_NO("Unable to get the audio RTP socket (IP=%s, port=%d)", media_ip, media_port); } /* create a second socket for video */ if((media_socket_video = socket(media_ip_is_ipv6 ? AF_INET6 : AF_INET, SOCK_DGRAM, 0)) == -1) { - char msg[512]; - sprintf(msg, "Unable to get the video RTP socket (IP=%s, port=%d)", media_ip, media_port+2); - ERROR_NO(msg); + ERROR_NO("Unable to get the video RTP socket (IP=%s, port=%d)", media_ip, media_port + 2); } int try_counter; @@ -2176,11 +2185,9 @@ int main(int argc, char *argv[]) for (try_counter = 0; try_counter < max_tries; try_counter++) { if (media_sockaddr.ss_family == AF_INET) { - (_RCAST(struct sockaddr_in *,&media_sockaddr))->sin_port = - htons((short)media_port); + (_RCAST(struct sockaddr_in *,&media_sockaddr))->sin_port = htons(media_port); } else { - (_RCAST(struct sockaddr_in6 *,&media_sockaddr))->sin6_port = - htons((short)media_port); + (_RCAST(struct sockaddr_in6 *,&media_sockaddr))->sin6_port = htons(media_port); media_ip_is_ipv6 = true; } // Use get_host_and_port to remove square brackets from an @@ -2188,7 +2195,7 @@ int main(int argc, char *argv[]) get_host_and_port(media_ip, media_ip_escaped, NULL); if(bind(media_socket, - (sockaddr *)(void *)&media_sockaddr, + (sockaddr*)&media_sockaddr, SOCK_ADDR_SIZE(&media_sockaddr)) == 0) { break; } @@ -2197,9 +2204,7 @@ int main(int argc, char *argv[]) } if (try_counter >= max_tries) { - char msg[512]; - sprintf(msg, "Unable to bind audio RTP socket (IP=%s, port=%d)", media_ip, media_port); - ERROR_NO(msg); + ERROR_NO("Unable to bind audio RTP socket (IP=%s, port=%d)", media_ip, media_port); } /*--------------------------------------------------------- @@ -2208,14 +2213,12 @@ int main(int argc, char *argv[]) ----------------------------------------------------------*/ if (media_sockaddr.ss_family == AF_INET) { - (_RCAST(struct sockaddr_in *,&media_sockaddr))->sin_port = - htons((short)media_port+2); + (_RCAST(struct sockaddr_in*, &media_sockaddr))->sin_port = htons(media_port + 2); // Use get_host_and_port to remove square brackets from an // IPv6 address get_host_and_port(media_ip, media_ip_escaped, NULL); } else { - (_RCAST(struct sockaddr_in6 *,&media_sockaddr))->sin6_port = - htons((short)media_port+2); + (_RCAST(struct sockaddr_in6*, &media_sockaddr))->sin6_port = htons(media_port + 2); media_ip_is_ipv6 = true; // Use get_host_and_port to remove square brackets from an // IPv6 address @@ -2223,16 +2226,14 @@ int main(int argc, char *argv[]) } if(bind(media_socket_video, - (sockaddr *)(void *)&media_sockaddr, + (sockaddr*)&media_sockaddr, SOCK_ADDR_SIZE(&media_sockaddr))) { - char msg[512]; - sprintf(msg, "Unable to bind video RTP socket (IP=%s, port=%d)", media_ip, media_port+2); - ERROR_NO(msg); + ERROR_NO("Unable to bind video RTP socket (IP=%s, port=%d)", media_ip, media_port + 2); } /* Second socket bound */ } - /* Creating the remote control socket thread */ + /* Creating the remote control socket thread. */ setup_ctrl_socket(); if (!nostdin) { setup_stdin_socket(); @@ -2243,20 +2244,19 @@ int main(int argc, char *argv[]) (&pthread2_id, NULL, (void *(*)(void *)) rtp_echo_thread, - (void*)&media_socket) + &media_socket) == -1) { ERROR_NO("Unable to create RTP echo thread"); } } - - /* Creating second RTP echo thread for video */ + /* Creating second RTP echo thread for video. */ if ((media_socket_video > 0) && (rtp_echo_enabled)) { if (pthread_create (&pthread3_id, NULL, (void *(*)(void *)) rtp_echo_thread, - (void*)&media_socket_video) + &media_socket_video) == -1) { ERROR_NO("Unable to create second RTP echo thread"); } @@ -2264,11 +2264,34 @@ int main(int argc, char *argv[]) traffic_thread(); + /* Cancel and join other threads. */ + if (pthread2_id) { + pthread_cancel(pthread2_id); + } + if (pthread3_id) { + pthread_cancel(pthread3_id); + } + if (pthread2_id) { + pthread_join(pthread2_id, NULL); + } + if (pthread3_id) { + pthread_join(pthread3_id, NULL); + } + + if (!nostdin) { + sipp_close_socket(stdin_socket); + } + sipp_close_socket(ctrl_socket); + #ifdef HAVE_EPOLL close(epollfd); free(epollevents); #endif + if (local_addr_storage) { + freeaddrinfo(local_addr_storage); + } free(scenario_file); - scenario_file = NULL; + + sipp_exit(EXIT_TEST_RES_UNKNOWN); } diff --git a/src/socket.cpp b/src/socket.cpp index 2a8c63bee..974658ca3 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -335,24 +335,13 @@ const char *sip_tls_error_string(SSL *ssl, int size) #endif -char* get_inet_address(struct sockaddr_storage* addr) +static char* get_inet_address(const struct sockaddr_storage* addr, char* dst, int len) { - static char* ip_addr = NULL; - - if (!ip_addr) { - ip_addr = (char *)malloc(1024*sizeof(char)); - } - if (getnameinfo(_RCAST(struct sockaddr *, addr), - SOCK_ADDR_SIZE(addr), - ip_addr, - 1024, - NULL, - 0, - NI_NUMERICHOST) != 0) { - strcpy(ip_addr, "addr not supported"); + if (getnameinfo(_RCAST(struct sockaddr*, addr), sizeof(*addr), + dst, len, NULL, 0, NI_NUMERICHOST) != 0) { + snprintf(dst, len, "addr not supported"); } - - return ip_addr; + return dst; } static bool process_key(int c) @@ -1377,7 +1366,7 @@ struct sipp_socket *new_sipp_socket(bool use_ipv6, int transport) { } #endif - ret = sipp_allocate_socket(use_ipv6, transport, fd); + ret = sipp_allocate_socket(use_ipv6, transport, fd); if (!ret) { close(fd); ERROR("Could not allocate new socket structure!"); @@ -1452,7 +1441,7 @@ struct sipp_socket *sipp_accept_socket(struct sipp_socket *accept_socket) { #endif - ret = sipp_allocate_socket(accept_socket->ss_ipv6, accept_socket->ss_transport, fd, 1); + ret = sipp_allocate_socket(accept_socket->ss_ipv6, accept_socket->ss_transport, fd, 1); if (!ret) { close(fd); ERROR_NO("Could not allocate new socket!"); @@ -1734,13 +1723,13 @@ static size_t decompress_if_needed(int sock, char* buff, size_t len, void** st) return 0; case COMP_DISCARD: - TRACE_MSG("Compressed message discarded by pluggin.\n"); + TRACE_MSG("Compressed message discarded by plugin.\n"); resynch_recv++; return 0; default: case COMP_KO: - ERROR("Compression pluggin error"); + ERROR("Compression plugin error"); return 0; } } @@ -2318,7 +2307,7 @@ static ssize_t socket_write_primitive(struct sipp_socket* socket, const char* bu if (comp_compress(&socket->ss_comp_state, comp_msg, (unsigned int *) &len) != COMP_OK) { - ERROR("Compression pluggin error"); + ERROR("Compression plugin error"); } buffer = (char *)comp_msg; @@ -2528,14 +2517,11 @@ int open_connections() } memset(&remote_sockaddr, 0, sizeof( remote_sockaddr )); - memcpy(&remote_sockaddr, - local_addr->ai_addr, - SOCK_ADDR_SIZE( - _RCAST(struct sockaddr_storage *, local_addr->ai_addr))); - + memcpy(&remote_sockaddr, local_addr->ai_addr, + SOCK_ADDR_SIZE(_RCAST(struct sockaddr_storage*, local_addr->ai_addr))); freeaddrinfo(local_addr); - strcpy(remote_ip, get_inet_address(&remote_sockaddr)); + get_inet_address(&remote_sockaddr, remote_ip, sizeof(remote_ip)); if (remote_sockaddr.ss_family == AF_INET) { (_RCAST(struct sockaddr_in *, &remote_sockaddr))->sin_port = htons((short)remote_port); @@ -2581,9 +2567,8 @@ int open_connections() local_sockaddr.ss_family = local_addr->ai_addr->sa_family; if (!strlen(local_ip)) { - strcpy(local_ip, - get_inet_address( - _RCAST(struct sockaddr_storage *, local_addr->ai_addr))); + get_inet_address(_RCAST(struct sockaddr_storage*, local_addr->ai_addr), + local_ip, sizeof(local_ip)); } else { memcpy(&local_sockaddr, local_addr->ai_addr, @@ -2881,7 +2866,8 @@ void connect_to_peer(char *peer_host, int peer_port, struct sockaddr_storage *pe htons((short)peer_port); is_ipv6 = true; } - strcpy(peer_ip, get_inet_address(peer_sockaddr)); + get_inet_address(peer_sockaddr, peer_ip, sizeof(peer_ip)); + if ((*peer_socket = new_sipp_socket(is_ipv6, T_TCP)) == NULL) { ERROR_NO("Unable to get a twin sipp TCP socket"); } @@ -2963,7 +2949,8 @@ void connect_local_twin_socket(char * twinSippHost) memcpy(&twinSipp_sockaddr, local_addr->ai_addr, SOCK_ADDR_SIZE( - _RCAST(struct sockaddr_storage *, local_addr->ai_addr))); + _RCAST(struct sockaddr_storage*, local_addr->ai_addr))); + freeaddrinfo(local_addr); if (twinSipp_sockaddr.ss_family == AF_INET) { (_RCAST(struct sockaddr_in *, &twinSipp_sockaddr))->sin_port = @@ -2973,7 +2960,7 @@ void connect_local_twin_socket(char * twinSippHost) htons((short)twinSippPort); is_ipv6 = true; } - strcpy(twinSippIp, get_inet_address(&twinSipp_sockaddr)); + get_inet_address(&twinSipp_sockaddr, twinSippIp, sizeof(twinSippIp)); if ((localTwinSippSocket = new_sipp_socket(is_ipv6, T_TCP)) == NULL) { ERROR_NO("Unable to get a listener TCP socket ");