From: Costa Tsaousis (ktsaou) Date: Wed, 1 Mar 2017 00:45:20 +0000 (+0200) Subject: fixes identified by coverity X-Git-Tag: ab-debian_0.20170311.01-0ab1~1^2~16^2~4 X-Git-Url: https://arthur.barton.de/gitweb/?p=netdata.git;a=commitdiff_plain;h=a4be7b4e24233a66f47a428b0cd256993328352b;ds=sidebyside fixes identified by coverity --- diff --git a/src/plugin_tc.c b/src/plugin_tc.c index 35763d31..7dcfedb3 100644 --- a/src/plugin_tc.c +++ b/src/plugin_tc.c @@ -1003,7 +1003,7 @@ void *tc_main(void *ptr) { rrddim_set(stcpu, "system", thread.ru_stime.tv_sec * 1000000ULL + thread.ru_stime.tv_usec); rrdset_done(stcpu); - if(unlikely(!sttime)) stcpu = rrdset_find_localhost("netdata.plugin_tc_time"); + if(unlikely(!sttime)) sttime = rrdset_find_localhost("netdata.plugin_tc_time"); if(unlikely(!sttime)) { sttime = rrdset_create_localhost("netdata", "plugin_tc_time", NULL, "tc.helper", NULL , "NetData TC script execution", "milliseconds/run", 135001 diff --git a/src/rrdcalctemplate.c b/src/rrdcalctemplate.c index 1deec43a..2c5e2bd1 100644 --- a/src/rrdcalctemplate.c +++ b/src/rrdcalctemplate.c @@ -23,6 +23,8 @@ void rrdcalctemplate_link_matching(RRDSET *st) { } inline void rrdcalctemplate_free(RRDHOST *host, RRDCALCTEMPLATE *rt) { + if(unlikely(!rt)) return; + debug(D_HEALTH, "Health removing template '%s' of host '%s'", rt->name, host->hostname); if(host->templates == rt) { diff --git a/src/rrdhost.c b/src/rrdhost.c index 7002c482..c5bc6bd7 100644 --- a/src/rrdhost.c +++ b/src/rrdhost.c @@ -409,6 +409,10 @@ void rrdhost_free(RRDHOST *host) { info("Freeing all memory for host '%s'...", host->hostname); rrd_check_wrlock(); // make sure the RRDs are write locked + + // stop a possibly running thread + rrdpush_sender_thread_stop(host); + rrdhost_wrlock(host); // lock this RRDHOST // ------------------------------------------------------------------------ @@ -447,8 +451,6 @@ void rrdhost_free(RRDHOST *host) { // ------------------------------------------------------------------------ // free it - rrdpush_sender_thread_stop(host); - freez(host->os); freez(host->cache_dir); freez(host->varlib_dir); diff --git a/src/rrdpush.c b/src/rrdpush.c index 06695c6f..07fcfead 100644 --- a/src/rrdpush.c +++ b/src/rrdpush.c @@ -185,31 +185,18 @@ static void rrdpush_sender_thread_reset_all_charts(RRDHOST *host) { static inline void rrdpush_sender_thread_data_flush(RRDHOST *host) { rrdpush_lock(host); + if(buffer_strlen(host->rrdpush_buffer)) error("STREAM %s [send]: discarding %zu bytes of metrics already in the buffer.", host->hostname, buffer_strlen(host->rrdpush_buffer)); buffer_flush(host->rrdpush_buffer); - rrdpush_sender_thread_reset_all_charts(host); - rrdpush_unlock(host); -} - -static inline void rrdpush_sender_thread_lock(RRDHOST *host) { - if(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0) - error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname); - rrdpush_lock(host); -} - -static inline void rrdpush_sender_thread_unlock(RRDHOST *host) { - if(pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0) - error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname); + rrdpush_sender_thread_reset_all_charts(host); rrdpush_unlock(host); } -static void rrdpush_sender_thread_cleanup(RRDHOST *host) { - rrdpush_lock(host); - +static void rrdpush_sender_thread_cleanup_locked_all(RRDHOST *host) { host->rrdpush_connected = 0; if(host->rrdpush_socket != -1) { @@ -232,18 +219,20 @@ static void rrdpush_sender_thread_cleanup(RRDHOST *host) { host->rrdpush_buffer = NULL; host->rrdpush_spawn = 0; - - rrdpush_unlock(host); } void rrdpush_sender_thread_stop(RRDHOST *host) { - rrdhost_check_wrlock(host); + rrdpush_lock(host); + rrdhost_wrlock(host); if(host->rrdpush_spawn) { info("STREAM %s [send]: stopping sending thread...", host->hostname); pthread_cancel(host->rrdpush_thread); - rrdpush_sender_thread_cleanup(host); + rrdpush_sender_thread_cleanup_locked_all(host); } + + rrdhost_unlock(host); + rrdpush_unlock(host); } void *rrdpush_sender_thread(void *ptr) { @@ -398,7 +387,19 @@ void *rrdpush_sender_thread(void *ptr) { } if(ofd->revents & POLLOUT && begin < buffer_strlen(host->rrdpush_buffer)) { - rrdpush_sender_thread_lock(host); + + // BEGIN RRDPUSH LOCKED SESSION + + // during this session, data collectors + // will not be able to append data to our buffer + // but the socket is in non-blocking mode + // so, we will not block at send() + + if(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0) + error("STREAM %s [send]: cannot set pthread cancel state to DISABLE.", host->hostname); + + rrdpush_lock(host); + ssize_t ret = send(host->rrdpush_socket, &host->rrdpush_buffer->buffer[begin], buffer_strlen(host->rrdpush_buffer) - begin, MSG_DONTWAIT); if(ret == -1) { if(errno != EAGAIN && errno != EINTR) { @@ -412,11 +413,19 @@ void *rrdpush_sender_thread(void *ptr) { sent_bytes += ret; begin += ret; if(begin == buffer_strlen(host->rrdpush_buffer)) { + // we send it all + buffer_flush(host->rrdpush_buffer); begin = 0; } } - rrdpush_sender_thread_unlock(host); + + rrdpush_unlock(host); + + if(pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0) + error("STREAM %s [send]: cannot set pthread cancel state to ENABLE.", host->hostname); + + // END RRDPUSH LOCKED SESSION } // protection from overflow @@ -433,7 +442,11 @@ void *rrdpush_sender_thread(void *ptr) { cleanup: debug(D_WEB_CLIENT, "STREAM %s [send]: sending thread exits.", host->hostname); - rrdpush_sender_thread_cleanup(host); + rrdpush_lock(host); + rrdhost_wrlock(host); + rrdpush_sender_thread_cleanup_locked_all(host); + rrdhost_unlock(host); + rrdpush_unlock(host); pthread_exit(NULL); return NULL; @@ -534,6 +547,7 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m info("STREAM %s [receive from [%s]:%s]: initializing communication...", host->hostname, client_ip, client_port); if(send_timeout(fd, START_STREAMING_PROMPT, strlen(START_STREAMING_PROMPT), 0, 60) != strlen(START_STREAMING_PROMPT)) { error("STREAM %s [receive from [%s]:%s]: cannot send ready command.", host->hostname, client_ip, client_port); + close(fd); return 0; } @@ -545,6 +559,7 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m FILE *fp = fdopen(fd, "r"); if(!fp) { error("STREAM %s [receive from [%s]:%s]: failed to get a FILE for FD %d.", host->hostname, client_ip, client_port, fd); + close(fd); return 0; } @@ -566,11 +581,11 @@ int rrdpush_receive(int fd, const char *key, const char *hostname, const char *m host->health_enabled = 0; host->senders_disconnected_time = now_realtime_sec(); - - rrdpush_sender_thread_stop(host); } rrdhost_unlock(host); + rrdpush_sender_thread_stop(host); + // cleanup fclose(fp); @@ -602,7 +617,6 @@ void *rrdpush_receiver_thread(void *ptr) { rrdpush_receive(rpt->fd, rpt->key, rpt->hostname, rpt->machine_guid, rpt->os, rpt->update_every, rpt->client_ip, rpt->client_port); info("STREAM %s [receive from [%s]:%s]: receive thread ended (task id %d)", rpt->hostname, rpt->client_ip, rpt->client_port, gettid()); - close(rpt->fd); freez(rpt->key); freez(rpt->hostname); freez(rpt->machine_guid); @@ -616,13 +630,19 @@ void *rrdpush_receiver_thread(void *ptr) { } void rrdpush_sender_thread_spawn(RRDHOST *host) { - if(pthread_create(&host->rrdpush_thread, NULL, rrdpush_sender_thread, (void *)host)) - error("STREAM %s [send]: failed to create new thread for client.", host->hostname); + rrdhost_wrlock(host); + + if(!host->rrdpush_spawn) { + if(pthread_create(&host->rrdpush_thread, NULL, rrdpush_sender_thread, (void *) host)) + error("STREAM %s [send]: failed to create new thread for client.", host->hostname); - else if(pthread_detach(host->rrdpush_thread)) - error("STREAM %s [send]: cannot request detach newly created thread.", host->hostname); + else if(pthread_detach(host->rrdpush_thread)) + error("STREAM %s [send]: cannot request detach newly created thread.", host->hostname); - host->rrdpush_spawn = 1; + host->rrdpush_spawn = 1; + } + + rrdhost_unlock(host); } int rrdpush_receiver_thread_spawn(RRDHOST *host, struct web_client *w, char *url) { @@ -630,7 +650,7 @@ int rrdpush_receiver_thread_spawn(RRDHOST *host, struct web_client *w, char *url info("STREAM [receive from [%s]:%s]: new client connection.", w->client_ip, w->client_port); - char *key = NULL, *hostname = NULL, *machine_guid = NULL, *os = NULL; + char *key = NULL, *hostname = NULL, *machine_guid = NULL, *os = "unknown"; int update_every = default_rrd_update_every; char buf[GUID_LEN + 1]; diff --git a/src/sys_fs_cgroup.c b/src/sys_fs_cgroup.c index 248f3d62..088495a6 100644 --- a/src/sys_fs_cgroup.c +++ b/src/sys_fs_cgroup.c @@ -466,8 +466,8 @@ static inline void cgroup_read_cpuacct_usage(struct cpuacct_usage *ca) { unsigned long i = procfile_linewords(ff, 0); if(unlikely(i == 0)) { - return; ca->updated = 0; + return; } // we may have 1 more CPU reported @@ -2113,7 +2113,7 @@ void update_cgroup_charts(int update_every) { ); for(i = 0; i < cg->cpuacct_usage.cpus; i++) { - snprintfz(id, CHART_TITLE_MAX, "cpu%u", i); + snprintfz(id, RRD_ID_LENGTH_MAX, "cpu%u", i); rrddim_add(cg->st_cpu_per_core, id, NULL, 100, 1000000000, RRD_ALGORITHM_INCREMENTAL); } } @@ -2121,7 +2121,7 @@ void update_cgroup_charts(int update_every) { rrdset_next(cg->st_cpu_per_core); for(i = 0; i < cg->cpuacct_usage.cpus ;i++) { - snprintfz(id, CHART_TITLE_MAX, "cpu%u", i); + snprintfz(id, RRD_ID_LENGTH_MAX, "cpu%u", i); rrddim_set(cg->st_cpu_per_core, id, cg->cpuacct_usage.cpu_percpu[i]); } rrdset_done(cg->st_cpu_per_core);