diff -Naurp pcp-5.3.7.orig/qa/1985 pcp-5.3.7/qa/1985 --- pcp-5.3.7.orig/qa/1985 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1985 2022-10-19 21:32:03.971832371 +1100 @@ -0,0 +1,38 @@ +#!/bin/sh +# PCP QA Test No. 1985 +# Exercise a pmfind fix - valgrind-enabled variant. +# +# Copyright (c) 2022 Red Hat. All Rights Reserved. +# + +seq=`basename $0` +echo "QA output created by $seq" + +# get standard environment, filters and checks +. ./common.product +. ./common.filter +. ./common.check + +_check_valgrind + +_cleanup() +{ + cd $here + $sudo rm -rf $tmp $tmp.* +} + +status=0 # success is the default! +$sudo rm -rf $tmp $tmp.* $seq.full +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# real QA test starts here +export seq +./1986 --valgrind \ +| $PCP_AWK_PROG ' +skip == 1 && $1 == "===" { skip = 0 } +/^=== std err ===/ { skip = 1 } +skip == 0 { print } +skip == 1 { print >>"'$here/$seq.full'" }' + +# success, all done +exit diff -Naurp pcp-5.3.7.orig/qa/1985.out pcp-5.3.7/qa/1985.out --- pcp-5.3.7.orig/qa/1985.out 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1985.out 2022-10-19 21:32:03.971832371 +1100 @@ -0,0 +1,11 @@ +QA output created by 1985 +QA output created by 1986 --valgrind +=== std out === +SOURCE HOSTNAME +=== filtered valgrind report === +Memcheck, a memory error detector +Command: pmfind -S -m probe=127.0.0.1/32 +LEAK SUMMARY: +definitely lost: 0 bytes in 0 blocks +indirectly lost: 0 bytes in 0 blocks +ERROR SUMMARY: 0 errors from 0 contexts ... diff -Naurp pcp-5.3.7.orig/qa/1986 pcp-5.3.7/qa/1986 --- pcp-5.3.7.orig/qa/1986 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1986 2022-10-19 21:32:03.971832371 +1100 @@ -0,0 +1,62 @@ +#!/bin/sh +# PCP QA Test No. 1986 +# Exercise libpcp_web timers pmfind regression fix. +# +# Copyright (c) 2022 Red Hat. All Rights Reserved. +# + +if [ $# -eq 0 ] +then + seq=`basename $0` + echo "QA output created by $seq" +else + # use $seq from caller, unless not set + [ -n "$seq" ] || seq=`basename $0` + echo "QA output created by `basename $0` $*" +fi + +# get standard environment, filters and checks +. ./common.product +. ./common.filter +. ./common.check + +do_valgrind=false +if [ "$1" = "--valgrind" ] +then + _check_valgrind + do_valgrind=true +fi + +test -x $PCP_BIN_DIR/pmfind || _notrun No support for pmfind + +_cleanup() +{ + cd $here + $sudo rm -rf $tmp $tmp.* +} + +status=0 # success is the default! +hostname=`hostname || echo localhost` +$sudo rm -rf $tmp $tmp.* $seq.full +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_filter() +{ + sed \ + -e "s@$tmp@TMP@g" \ + -e "s/ $hostname/ HOSTNAME/" \ + -e 's/^[a-f0-9][a-f0-9]* /SOURCE /' \ + # end +} + +# real QA test starts here +if $do_valgrind +then + _run_valgrind pmfind -S -m probe=127.0.0.1/32 +else + pmfind -S -m probe=127.0.0.1/32 +fi \ +| _filter + +# success, all done +exit diff -Naurp pcp-5.3.7.orig/qa/1986.out pcp-5.3.7/qa/1986.out --- pcp-5.3.7.orig/qa/1986.out 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1986.out 2022-10-19 21:32:03.971832371 +1100 @@ -0,0 +1,2 @@ +QA output created by 1986 +SOURCE HOSTNAME diff -Naurp pcp-5.3.7.orig/qa/group pcp-5.3.7/qa/group --- pcp-5.3.7.orig/qa/group 2022-10-19 20:49:42.638708707 +1100 +++ pcp-5.3.7/qa/group 2022-10-19 21:32:03.972832359 +1100 @@ -1974,4 +1974,6 @@ x11 1957 libpcp local valgrind 1978 atop local 1984 pmlogconf pmda.redis local +1985 pmfind local valgrind +1986 pmfind local 4751 libpcp threads valgrind local pcp helgrind diff -Naurp pcp-5.3.7.orig/src/libpcp_web/src/webgroup.c pcp-5.3.7/src/libpcp_web/src/webgroup.c --- pcp-5.3.7.orig/src/libpcp_web/src/webgroup.c 2021-11-01 13:02:26.000000000 +1100 +++ pcp-5.3.7/src/libpcp_web/src/webgroup.c 2022-10-19 21:32:03.973832346 +1100 @@ -287,11 +287,24 @@ webgroup_new_context(pmWebGroupSettings } static void +webgroup_timers_stop(struct webgroups *groups) +{ + if (groups->active) { + uv_timer_stop(&groups->timer); + uv_close((uv_handle_t *)&groups->timer, NULL); + pmWebTimerRelease(groups->timerid); + groups->timerid = -1; + groups->active = 0; + } +} + +static void webgroup_garbage_collect(struct webgroups *groups) { dictIterator *iterator; dictEntry *entry; context_t *cp; + unsigned int count = 0, drops = 0; if (pmDebugOptions.http || pmDebugOptions.libweb) fprintf(stderr, "%s: started\n", "webgroup_garbage_collect"); @@ -308,33 +321,40 @@ webgroup_garbage_collect(struct webgroup uv_mutex_unlock(&groups->mutex); webgroup_drop_context(cp, groups); uv_mutex_lock(&groups->mutex); + drops++; } + count++; } dictReleaseIterator(iterator); + + /* if dropping the last remaining context, do cleanup */ + if (groups->active && drops == count) { + if (pmDebugOptions.http || pmDebugOptions.libweb) + fprintf(stderr, "%s: freezing\n", "webgroup_garbage_collect"); + webgroup_timers_stop(groups); + } uv_mutex_unlock(&groups->mutex); } if (pmDebugOptions.http || pmDebugOptions.libweb) - fprintf(stderr, "%s: finished\n", "webgroup_garbage_collect"); + fprintf(stderr, "%s: finished [%u drops from %u entries]\n", + "webgroup_garbage_collect", drops, count); } static void refresh_maps_metrics(void *data) { struct webgroups *groups = (struct webgroups *)data; + unsigned int value; - if (groups->metrics) { - unsigned int value; - - value = dictSize(contextmap); - mmv_set(groups->map, groups->metrics[CONTEXT_MAP_SIZE], &value); - value = dictSize(namesmap); - mmv_set(groups->map, groups->metrics[NAMES_MAP_SIZE], &value); - value = dictSize(labelsmap); - mmv_set(groups->map, groups->metrics[LABELS_MAP_SIZE], &value); - value = dictSize(instmap); - mmv_set(groups->map, groups->metrics[INST_MAP_SIZE], &value); - } + value = contextmap? dictSize(contextmap) : 0; + mmv_set(groups->map, groups->metrics[CONTEXT_MAP_SIZE], &value); + value = namesmap? dictSize(namesmap) : 0; + mmv_set(groups->map, groups->metrics[NAMES_MAP_SIZE], &value); + value = labelsmap? dictSize(labelsmap) : 0; + mmv_set(groups->map, groups->metrics[LABELS_MAP_SIZE], &value); + value = instmap? dictSize(instmap) : 0; + mmv_set(groups->map, groups->metrics[INST_MAP_SIZE], &value); } static void @@ -487,6 +507,7 @@ pmWebGroupDestroy(pmWebGroupSettings *se if (pmDebugOptions.libweb) fprintf(stderr, "%s: destroy context %p gp=%p\n", "pmWebGroupDestroy", cp, gp); + webgroup_deref_context(cp); webgroup_drop_context(cp, gp); } sdsfree(msg); @@ -2394,17 +2415,12 @@ pmWebGroupClose(pmWebGroupModule *module if (groups) { /* walk the contexts, stop timers and free resources */ - if (groups->active) { - groups->active = 0; - uv_timer_stop(&groups->timer); - pmWebTimerRelease(groups->timerid); - groups->timerid = -1; - } iterator = dictGetIterator(groups->contexts); while ((entry = dictNext(iterator)) != NULL) webgroup_drop_context((context_t *)dictGetVal(entry), NULL); dictReleaseIterator(iterator); dictRelease(groups->contexts); + webgroup_timers_stop(groups); memset(groups, 0, sizeof(struct webgroups)); free(groups); } diff -Naurp pcp-5.3.7.orig/src/pmfind/source.c pcp-5.3.7/src/pmfind/source.c --- pcp-5.3.7.orig/src/pmfind/source.c 2021-02-17 15:27:41.000000000 +1100 +++ pcp-5.3.7/src/pmfind/source.c 2022-10-19 21:32:03.973832346 +1100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Red Hat. + * Copyright (c) 2020,2022 Red Hat. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -25,6 +25,7 @@ static pmWebGroupSettings settings; typedef struct { sds source; sds hostspec; + unsigned int refcount; } context_t; typedef struct { @@ -38,22 +39,34 @@ typedef struct { } sources_t; static void +source_release(sources_t *sp, context_t *cp, sds ctx) +{ + pmWebGroupDestroy(&settings, ctx, sp); + sdsfree(cp->hostspec); + sdsfree(cp->source); + free(cp); +} + +static void sources_release(void *arg, const struct dictEntry *entry) { sources_t *sp = (sources_t *)arg; context_t *cp = (context_t *)dictGetVal(entry); sds ctx = (sds)entry->key; - pmWebGroupDestroy(&settings, ctx, sp); - sdsfree(cp->hostspec); - sdsfree(cp->source); + if (pmDebugOptions.discovery) + fprintf(stderr, "releasing context %s\n", ctx); + + source_release(sp, cp, ctx); } static void -sources_containers(sources_t *sp, sds id, dictEntry *uniq) +sources_containers(sources_t *sp, context_t *cp, sds id, dictEntry *uniq) { uv_mutex_lock(&sp->mutex); - sp->count++; /* issuing another PMWEBAPI request */ + /* issuing another PMWEBAPI request */ + sp->count++; + cp->refcount++; uv_mutex_unlock(&sp->mutex); pmWebGroupScrape(&settings, id, sp->params, sp); @@ -75,6 +88,7 @@ on_source_context(sds id, pmWebSource *s cp->source = sdsdup(src->source); cp->hostspec = sdsdup(src->hostspec); + cp->refcount = 1; uv_mutex_lock(&sp->mutex); dictAdd(sp->contexts, id, cp); @@ -84,7 +98,7 @@ on_source_context(sds id, pmWebSource *s if (entry) { /* source just discovered */ printf("%s %s\n", src->source, src->hostspec); if (containers) - sources_containers(sp, id, entry); + sources_containers(sp, cp, id, entry); } } @@ -116,7 +130,9 @@ static void on_source_done(sds context, int status, sds message, void *arg) { sources_t *sp = (sources_t *)arg; - int count = 0, release = 0; + context_t *cp; + dictEntry *he; + int remove = 0, count = 0, release = 0; if (pmDebugOptions.discovery) fprintf(stderr, "done on context %s (sts=%d)\n", context, status); @@ -127,19 +143,26 @@ on_source_done(sds context, int status, uv_mutex_lock(&sp->mutex); if ((count = --sp->count) <= 0) release = 1; + if ((he = dictFind(sp->contexts, context)) != NULL && + (cp = (context_t *)dictGetVal(he)) != NULL && + (--cp->refcount <= 0)) + remove = 1; uv_mutex_unlock(&sp->mutex); + if (remove) { + if (pmDebugOptions.discovery) + fprintf(stderr, "remove context %s\n", context); + source_release(sp, cp, context); + dictDelete(sp->contexts, context); + } + if (release) { unsigned long cursor = 0; - - if (pmDebugOptions.discovery) - fprintf(stderr, "release context %s (sts=%d)\n", context, status); do { cursor = dictScan(sp->contexts, cursor, sources_release, NULL, sp); } while (cursor); - } else { - if (pmDebugOptions.discovery) - fprintf(stderr, "not yet releasing (count=%d)\n", count); + } else if (pmDebugOptions.discovery) { + fprintf(stderr, "not yet releasing (count=%d)\n", count); } } @@ -190,6 +213,7 @@ sources_discovery_start(uv_timer_t *arg) } dictRelease(dp); + pmWebTimerClose(); } /* @@ -214,8 +238,8 @@ source_discovery(int count, char **urls) uv_mutex_init(&find.mutex); find.urls = urls; find.count = count; /* at least one PMWEBAPI request for each url */ - find.uniq = dictCreate(&sdsDictCallBacks, NULL); - find.params = dictCreate(&sdsDictCallBacks, NULL); + find.uniq = dictCreate(&sdsKeyDictCallBacks, NULL); + find.params = dictCreate(&sdsOwnDictCallBacks, NULL); dictAdd(find.params, sdsnew("name"), sdsnew("containers.state.running")); find.contexts = dictCreate(&sdsKeyDictCallBacks, NULL); @@ -230,6 +254,7 @@ source_discovery(int count, char **urls) pmWebGroupSetup(&settings.module); pmWebGroupSetEventLoop(&settings.module, loop); + pmWebTimerSetEventLoop(loop); /* * Start a one-shot timer to add a start function into the loop @@ -244,7 +269,9 @@ source_discovery(int count, char **urls) /* * Finished, release all resources acquired so far */ + pmWebGroupClose(&settings.module); uv_mutex_destroy(&find.mutex); + dictRelease(find.uniq); dictRelease(find.params); dictRelease(find.contexts); return find.status; diff -Naurp pcp-5.3.7.orig/src/pmproxy/src/server.c pcp-5.3.7/src/pmproxy/src/server.c --- pcp-5.3.7.orig/src/pmproxy/src/server.c 2022-04-05 09:05:43.000000000 +1000 +++ pcp-5.3.7/src/pmproxy/src/server.c 2022-10-19 21:31:43.831093354 +1100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2019,2021 Red Hat. + * Copyright (c) 2018-2019,2021-2022 Red Hat. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -310,17 +310,21 @@ on_write_callback(uv_callback_t *handle, struct client *client = (struct client *)request->writer.data; int sts; + (void)handle; if (pmDebugOptions.af) fprintf(stderr, "%s: client=%p\n", "on_write_callback", client); if (client->stream.secure == 0) { sts = uv_write(&request->writer, (uv_stream_t *)&client->stream, &request->buffer[0], request->nbuffers, request->callback); - if (sts != 0) - fprintf(stderr, "%s: ERROR uv_write failed\n", "on_write_callback"); + if (sts != 0) { + pmNotifyErr(LOG_ERR, "%s: %s - uv_write failed [%s]: %s\n", + pmGetProgname(), "on_write_callback", + uv_err_name(sts), uv_strerror(sts)); + client_close(client); + } } else secure_client_write(client, request); - (void)handle; return 0; } @@ -455,14 +459,16 @@ on_client_connection(uv_stream_t *stream uv_handle_t *handle; if (status != 0) { - fprintf(stderr, "%s: client connection failed: %s\n", - pmGetProgname(), uv_strerror(status)); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "on_client_connection", "connection", + uv_err_name(status), uv_strerror(status)); return; } if ((client = calloc(1, sizeof(*client))) == NULL) { - fprintf(stderr, "%s: out-of-memory for new client\n", - pmGetProgname()); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "on_client_connection", "calloc", + "ENOMEM", strerror(ENOMEM)); return; } if (pmDebugOptions.context | pmDebugOptions.af) @@ -476,16 +482,18 @@ on_client_connection(uv_stream_t *stream status = uv_tcp_init(proxy->events, &client->stream.u.tcp); if (status != 0) { - fprintf(stderr, "%s: client tcp init failed: %s\n", - pmGetProgname(), uv_strerror(status)); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "on_client_connection", "uv_tcp_init", + uv_err_name(status), uv_strerror(status)); client_put(client); return; } status = uv_accept(stream, (uv_stream_t *)&client->stream.u.tcp); if (status != 0) { - fprintf(stderr, "%s: client tcp init failed: %s\n", - pmGetProgname(), uv_strerror(status)); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "on_client_connection", "uv_accept", + uv_err_name(status), uv_strerror(status)); client_put(client); return; } @@ -496,8 +504,9 @@ on_client_connection(uv_stream_t *stream status = uv_read_start((uv_stream_t *)&client->stream.u.tcp, on_buffer_alloc, on_client_read); if (status != 0) { - fprintf(stderr, "%s: client read start failed: %s\n", - pmGetProgname(), uv_strerror(status)); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "on_client_connection", "uv_read_start", + uv_err_name(status), uv_strerror(status)); client_close(client); } } @@ -530,8 +539,9 @@ open_request_port(struct proxy *proxy, s sts = uv_listen((uv_stream_t *)&stream->u.tcp, maxpending, on_client_connection); if (sts != 0) { - fprintf(stderr, "%s: socket listen error %s\n", - pmGetProgname(), uv_strerror(sts)); + pmNotifyErr(LOG_ERR, "%s: %s - uv_listen failed [%s]: %s\n", + pmGetProgname(), "open_request_port", + uv_err_name(sts), uv_strerror(sts)); uv_close(handle, NULL); return -ENOTCONN; } @@ -554,15 +564,23 @@ open_request_local(struct proxy *proxy, uv_pipe_init(proxy->events, &stream->u.local, 0); handle = (uv_handle_t *)&stream->u.local; handle->data = (void *)proxy; - uv_pipe_bind(&stream->u.local, name); + sts = uv_pipe_bind(&stream->u.local, name); + if (sts != 0) { + pmNotifyErr(LOG_ERR, "%s: %s - uv_pipe_bind %s failed [%s]: %s\n", + pmGetProgname(), "open_request_local", name, + uv_err_name(sts), uv_strerror(sts)); + uv_close(handle, NULL); + return -ENOTCONN; + } #ifdef HAVE_UV_PIPE_CHMOD uv_pipe_chmod(&stream->u.local, UV_READABLE | UV_WRITABLE); #endif sts = uv_listen((uv_stream_t *)&stream->u.local, maxpending, on_client_connection); if (sts != 0) { - fprintf(stderr, "%s: local listen error %s\n", - pmGetProgname(), uv_strerror(sts)); + pmNotifyErr(LOG_ERR, "%s: %s - %s failed [%s]: %s\n", + pmGetProgname(), "open_request_local", "uv_listen", + uv_err_name(sts), uv_strerror(sts)); uv_close(handle, NULL); return -ENOTCONN; }