From bed7636e1ed7509a432c21f7e68420296ad1cf3b Mon Sep 17 00:00:00 2001 From: Paolo Fabio Zaino Date: Thu, 17 Mar 2022 18:19:03 +0000 Subject: [PATCH 1/4] Improved Makefile and added 'testing' target to build with sanitizers enabled, removed 3 memory leaks from test code, nothing important as they were in the test code, but it's useful to have better code examples for new users --- Makefile | 12 ++++++++++++ tests/02ITest004.c | 9 ++++++--- tests/04PTest005.c | 27 ++++++++++----------------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index 58d286a..03b2f98 100644 --- a/Makefile +++ b/Makefile @@ -230,14 +230,17 @@ LIBST:=$(LIBDIR)/lib$(LIBNAME).a ############################################################################### # Targets: +# Use "make all" to build both core library and tests for production .PHONY: all all: CFLAGS += $(P_CFLAGS) all: core test +# Use "make clean" to clean your previous builds .PHONY: clean clean: $(RM) -r $(LIBDIR) $(OBJ) $(TESTDIR)/bin ./*.o +# Use "make configure" to set your configuration (useful to enable code in your IDE) .PHONY: configure configure: $(SRC)/$(LIBNAME)_config.h $(SCRIPTSDIR)/ux_set_extension @echo ---------------------------------------------------------------- @@ -249,12 +252,14 @@ configure: $(SRC)/$(LIBNAME)_config.h $(SCRIPTSDIR)/ux_set_extension $(RVAL5) || : @echo ---------------------------------------------------------------- +# Use "make core" to just build the library FOR PRODUCTION .PHONY: core core: configure $(LIBDIR) $(LIBST) .PHONY: test test: $(TESTDIR)/bin $(TESTBINS) +# Use "make tests" to build tests and run them .PHONY: tests tests: test $(info ) @@ -263,11 +268,18 @@ tests: test $(info ===========================) for test in $(TESTBINS) ; do time ./$(TESTBIN)$$test ; done +# Use "make debug" to build code for gdb debugger .PHONY: debug debug: CFLAGS+= -ggdb3 debug: CODE_MACROS+= -DDEBUG debug: core tests +# Use "make testing" to build code with sanitizers for testing purposes: +.PHONY: testing +testing: CFLAGS+= -ggdb3 -fsanitize=address -fsanitize=leak -fsanitize=undefined +testing: core tests + +# Use "make install" to build and install the core library .PHONY: install install: core sudo cp -f $(LIBST) $(DESTDIR) diff --git a/tests/02ITest004.c b/tests/02ITest004.c index 56f6810..59b0ef6 100644 --- a/tests/02ITest004.c +++ b/tests/02ITest004.c @@ -139,8 +139,11 @@ void *consumer(void *arg) { uint32_t i; for (i = 0; i < MAX_ITEMS;) { - // For beginners: this is how in C we convert back a void * into the original dtata_type - QueueItem *item = (QueueItem *)malloc(sizeof(QueueItem *)); + // For beginners: this is how in C we convert back a void * + // into the original data_type: + QueueItem *item; // We do not need to allocate item, because + // ZVector vect_remove_front will do it for us :) + int fetched_item= 0; // Let's retrieve the value from the vector correctly: @@ -164,7 +167,7 @@ void *consumer(void *arg) { } free(item); - // item = NULL; + item = NULL; } printf("Consumer thread %i done. Consumed %d events.\n", id, evt_counter); diff --git a/tests/04PTest005.c b/tests/04PTest005.c index 225b12e..48cf7d0 100644 --- a/tests/04PTest005.c +++ b/tests/04PTest005.c @@ -130,12 +130,12 @@ void *producer(void *arg) { qi.msg[max_strLen]='\0'; // Enqueue message on the local queue: + // (this will copy qi into v2, so no need + // to free qi after, we can simply reuse it) vect_add(v2, &qi); } - // Now merge the local partition to the shared vector: - //vect_lock(v); - + // Now move the local partition to the shared vector: vect_move(v, v2, 0, MAX_ITEMS); // We're done, display some stats and terminate the thread: @@ -157,7 +157,10 @@ void *producer(void *arg) { #endif fflush(stdout); - //vect_unlock(v); + // Given that we have moved all the references to our message + // set from v2 to v, we can safely destroy v2 and recover its + // memory: + vect_destroy(v2); pthread_exit(NULL); return NULL; @@ -196,7 +199,6 @@ void *consumer(void *arg) { //if (vect_trylock(v)) //if (vect_wait_for_signal(v)) //{ - //vect_lock(v); //if ( vect_size(v) >= MAX_ITEMS ) if (!vect_move_if(v2, v, 0, MAX_ITEMS, check_if_correct_size)) { @@ -220,7 +222,7 @@ void *consumer(void *arg) { START_JOB: printf("--- Consumer Thread %*i received a chunk of %*i messages ---\n\n", 3, id, 4, vect_size(v2)); - QueueItem *item = (QueueItem *)malloc(sizeof(QueueItem *)); + QueueItem *item;// = (QueueItem *)malloc(sizeof(QueueItem *)); evt_counter = 0; for (i = 0; i < MAX_ITEMS; i++) @@ -285,7 +287,9 @@ int main() { int err = 0; int i = 0; struct thread_args *targs[MAX_THREADS+1]; + CCPAL_START_MEASURING; + for (i=0; i < MAX_THREADS / 2; i++) { targs[i]=(struct thread_args *)malloc(sizeof(struct thread_args)); targs[i]->id=i; @@ -322,20 +326,9 @@ int main() { CCPAL_REPORT_ANALYSIS; fflush(stdout); - // Allow the Producer and Consumer process - // To start properly: - //usleep(100); - printf("--- Events missed in the queue: %*i ---\n\n", 4, vect_size(v)); fflush(stdout); - // Now wait until the Queue is empty: - //while(!vect_is_empty(v)) - // ; - - //printf("--- Events missed in the queue: %*i ---\n", 4, vect_size(v)); - //fflush(stdout); - printf("Test %s_%d: Delete all left over events (if any):\n", testGrp, testID); fflush(stdout); From d571f5109fb9631024b70ce49611969fb9fef0a8 Mon Sep 17 00:00:00 2001 From: Paolo Fabio Zaino Date: Thu, 17 Mar 2022 18:46:46 +0000 Subject: [PATCH 2/4] fixed a wrong test ID number and cleaned a comment in another test --- tests/02ITest004.c | 4 ++-- tests/04PTest005.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/02ITest004.c b/tests/02ITest004.c index 59b0ef6..a27e0f5 100644 --- a/tests/02ITest004.c +++ b/tests/02ITest004.c @@ -34,8 +34,8 @@ #define MAX_MSG_SIZE 72 // Setup tests: -char *testGrp = "003"; -uint8_t testID = 4; +char *testGrp = "004"; +uint8_t testID = 1; #if ( ZVECT_THREAD_SAFE == 1 ) && ( OS_TYPE == 1 ) diff --git a/tests/04PTest005.c b/tests/04PTest005.c index 48cf7d0..eec3758 100644 --- a/tests/04PTest005.c +++ b/tests/04PTest005.c @@ -222,7 +222,8 @@ void *consumer(void *arg) { START_JOB: printf("--- Consumer Thread %*i received a chunk of %*i messages ---\n\n", 3, id, 4, vect_size(v2)); - QueueItem *item;// = (QueueItem *)malloc(sizeof(QueueItem *)); + QueueItem *item; // We do not need to allocate item, + // ZVector vect_remove_front will do it for us :) evt_counter = 0; for (i = 0; i < MAX_ITEMS; i++) From 497c2fa0ce212e599279d7cfa1ed763d3ca2df91 Mon Sep 17 00:00:00 2001 From: Paolo Fabio Zaino Date: Fri, 18 Mar 2022 01:55:09 +0000 Subject: [PATCH 3/4] fixed a memory leak in one of the tests and improved a bit the syntax in another test --- tests/02ITest004.c | 8 ++------ tests/04PTest005.c | 13 +++++++++++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/02ITest004.c b/tests/02ITest004.c index a27e0f5..c682fed 100644 --- a/tests/02ITest004.c +++ b/tests/02ITest004.c @@ -141,22 +141,18 @@ void *consumer(void *arg) { for (i = 0; i < MAX_ITEMS;) { // For beginners: this is how in C we convert back a void * // into the original data_type: - QueueItem *item; // We do not need to allocate item, because - // ZVector vect_remove_front will do it for us :) + QueueItem *item = NULL; // We do not need to allocate item, because + // ZVector vect_remove_front will do it for us :) int fetched_item= 0; // Let's retrieve the value from the vector correctly: - //vect_lock(v); - if (!vect_is_empty(v)) { item = (QueueItem *)vect_remove_front(v); fetched_item=1; } - //vect_unlock(v); - if ( fetched_item == 1 && item != NULL ) { // Let's test if the value we have retrieved is correct: diff --git a/tests/04PTest005.c b/tests/04PTest005.c index eec3758..9d5acef 100644 --- a/tests/04PTest005.c +++ b/tests/04PTest005.c @@ -59,7 +59,7 @@ size_t max_strLen = 32; // system when using multi-threaded queues. #define MAX_THREADS 16 -#define TOTAL_ITEMS 10000000 +#define TOTAL_ITEMS 100000000 #define MAX_ITEMS (TOTAL_ITEMS / ( MAX_THREADS / 2)) #define MAX_MSG_SIZE 72 pthread_t tid[MAX_THREADS]; // threads IDs @@ -230,7 +230,14 @@ void *consumer(void *arg) { { item = (QueueItem *)vect_remove_front(v2); if ( item->msg != NULL ) + { evt_counter++; + if (i < MAX_ITEMS - 1) + { + free(item); + item = NULL; + } + } #ifdef DEBUG // printf("thread %*i, item %*u\n", 10, id, 8, evt_counter); #endif @@ -241,7 +248,9 @@ void *consumer(void *arg) { printf("Last element in the queue for Consumer Thread %*i: ID (%*d) - Message: %s\n", 8, id, 8, item->eventID, item->msg); #endif - free(item); + + free(item); + item = NULL; printf("Consumer thread %i done. Consumed %d events.\n", id, evt_counter); From fc74281335953b7241fe012c42db9cf4fd0740c0 Mon Sep 17 00:00:00 2001 From: Paolo Fabio Zaino Date: Fri, 18 Mar 2022 01:56:47 +0000 Subject: [PATCH 4/4] reduced the number of items in 04PTest005.c from 100,000,000 to just 10,000,000 lol forgot the big number on previous tests and pushed it in github by mistake --- tests/04PTest005.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/04PTest005.c b/tests/04PTest005.c index 9d5acef..3a69fbb 100644 --- a/tests/04PTest005.c +++ b/tests/04PTest005.c @@ -59,7 +59,7 @@ size_t max_strLen = 32; // system when using multi-threaded queues. #define MAX_THREADS 16 -#define TOTAL_ITEMS 100000000 +#define TOTAL_ITEMS 10000000 #define MAX_ITEMS (TOTAL_ITEMS / ( MAX_THREADS / 2)) #define MAX_MSG_SIZE 72 pthread_t tid[MAX_THREADS]; // threads IDs