Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make improvements #383

Merged
merged 10 commits into from
Sep 27, 2023
Merged

Make improvements #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
76261ab
Optional BUILDDIR for output files
carlgsmith Sep 21, 2023
746ed75
Add gcov generation and output to testing
carlgsmith Sep 21, 2023
bc91406
Add Address Sanitizer to unit tests
carlgsmith Sep 22, 2023
da8e144
Remove duplicate global variables picked up by asan
carlgsmith Sep 22, 2023
fcf1d29
Do not run perf, fork or deadlock tests if running asan
carlgsmith Sep 25, 2023
38a264b
Fail on asan error and run asan fast stack unwind
carlgsmith Sep 25, 2023
9b8c6d1
Better check for socket still being used
carlgsmith Sep 25, 2023
5b9fb38
Reinstate old unittests without gcov and asan
carlgsmith Sep 25, 2023
1cb13dc
Remove unused -L when linking client executable
carlgsmith Sep 27, 2023
7d155f5
Wait until apteryxd exits before generating gcov output
carlgsmith Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pr-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- uses: actions/checkout@v3

- name: Install dependencies
run: sudo apt install -y libcunit1-dev liblua5.3-dev
run: sudo apt install -y libcunit1-dev liblua5.3-dev lcov

- name: Build
run: make
Expand Down
70 changes: 50 additions & 20 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ endif
DESTDIR?=./
PREFIX?=/usr/
LIBDIR?=lib
ifeq (test,$(firstword $(MAKECMDGOALS)))
BUILDDIR=.test
else
BUILDDIR?=.
endif
CC:=$(CROSS_COMPILE)gcc
LD:=$(CROSS_COMPILE)ld
PKG_CONFIG ?= pkg-config
Expand All @@ -23,7 +28,7 @@ ABI_VERSION=4
CFLAGS := $(CFLAGS) -g -O2
EXTRA_CFLAGS += -Wall -Wno-comment -std=c99 -D_GNU_SOURCE -fPIC
EXTRA_CFLAGS += -I. $(shell $(PKG_CONFIG) --cflags glib-2.0)
EXTRA_LDFLAGS := $(shell $(PKG_CONFIG) --libs glib-2.0) -lpthread
EXTRA_LDFLAGS := -L$(BUILDDIR) $(shell $(PKG_CONFIG) --libs glib-2.0) -lpthread
ifneq ($(HAVE_LUA),no)
LUAVERSION := $(shell $(PKG_CONFIG) --exists lua5.3 && echo lua5.3 ||\
($(PKG_CONFIG) --exists lua5.2 && echo lua5.2 ||\
Expand All @@ -40,62 +45,87 @@ apteryx_CFLAGS += -DTEST
apteryx_LDFLAGS += -lcunit
endif

all: libapteryx.so apteryx apteryxd
all: $(BUILDDIR)/libapteryx.so $(BUILDDIR)/apteryx $(BUILDDIR)/apteryxd

$(BUILDDIR):
@mkdir -p $@

libapteryx.so.$(ABI_VERSION): rpc.o rpc_transport.o rpc_socket.o apteryx.o lua.o
$(BUILDDIR)/libapteryx.so.$(ABI_VERSION): $(BUILDDIR)/rpc.o $(BUILDDIR)/rpc_transport.o $(BUILDDIR)/rpc_socket.o $(BUILDDIR)/apteryx.o $(BUILDDIR)/lua.o
@echo "Creating library "$@""
$(Q)$(CC) -shared $(LDFLAGS) -o $@ $^ $(EXTRA_LDFLAGS) -Wl,-soname,$@

libapteryx.so: libapteryx.so.$(ABI_VERSION)
@ln -s -f $@.$(ABI_VERSION) $@
@ln -s -f $@ apteryx.so
$(BUILDDIR)/libapteryx.so: $(BUILDDIR)/libapteryx.so.$(ABI_VERSION)
@ln -s -f libapteryx.so.$(ABI_VERSION) $@
@ln -s -f libapteryx.so $(BUILDDIR)/apteryx.so

%.o: %.c
$(BUILDDIR)/%.o: %.c | $(BUILDDIR)
@echo "Compiling "$<""
$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -c $< -o $@

apteryxd: apteryxd.c hashtree.c database.c rpc.o rpc_transport.o rpc_socket.o config.o callbacks.o libapteryx.so
$(BUILDDIR)/apteryxd: apteryxd.c hashtree.c database.c $(BUILDDIR)/rpc_transport.o $(BUILDDIR)/rpc_socket.o $(BUILDDIR)/config.o $(BUILDDIR)/callbacks.o $(BUILDDIR)/libapteryx.so
@echo "Building $@"
$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -o $@ $^ $(EXTRA_LDFLAGS)

apteryx: apteryxc.c hashtree.c database.c callbacks.c libapteryx.so $(EXTRA_CSRC)
$(BUILDDIR)/apteryx: apteryxc.c hashtree.c database.c callbacks.c $(BUILDDIR)/libapteryx.so $(EXTRA_CSRC)
@echo "Building $@"
$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(apteryx_CFLAGS) -o $@ $^ -L. -lapteryx $(EXTRA_LDFLAGS) $(apteryx_LDFLAGS)
$(Q)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $(apteryx_CFLAGS) -o $@ $^ -lapteryx $(EXTRA_LDFLAGS) $(apteryx_LDFLAGS)

apteryxd = \
if test -e /tmp/apteryxd.pid; then \
kill -TERM `cat /tmp/apteryxd.pid` && sleep 0.1; \
fi; \
rm -f /tmp/apteryxd.pid; \
rm -f /tmp/apteryxd.run; \
LD_LIBRARY_PATH=$(LD_LIBRARY_PATH):./ ./apteryxd -b -p /tmp/apteryxd.pid -r /tmp/apteryxd.run && sleep 0.1; \
LD_LIBRARY_PATH=$(LD_LIBRARY_PATH):./ $(TEST_WRAPPER) ./$(1); \
kill -TERM `cat /tmp/apteryxd.pid`;
export ASAN_OPTIONS=fast_unwind_on_malloc=true:halt_on_error=0:detect_stack_use_after_return=1:log_path=$(BUILDDIR)/asan-log; \
export LD_LIBRARY_PATH=$(LD_LIBRARY_PATH):$(BUILDDIR)/; \
export LUA_CPATH=$(BUILDDIR)/?.so; \
$(BUILDDIR)/apteryxd -b -p /tmp/apteryxd.pid -r /tmp/apteryxd.run && sleep 0.1; \
$(TEST_WRAPPER) $(BUILDDIR)/$(1); \
APID=`cat /tmp/apteryxd.pid`; \
kill -TERM $$APID; \
while kill -0 $$APID 2> /dev/null; do sleep 1; done;

ifeq (test,$(firstword $(MAKECMDGOALS)))
ifeq (uinttest,$(firstword $(MAKECMDGOALS)))
TEST_ARGS := $(wordlist 2,$(words $(MAKECMDGOALS)),$(MAKECMDGOALS))
$(eval $(TEST_ARGS):;@:)
endif

test: apteryxd apteryx
unittest: $(BUILDDIR)/apteryxd $(BUILDDIR)/apteryx
@echo "Running apteryx unit test: $<"
$(Q)$(call apteryxd,apteryx -u"$(TEST_ARGS)")
@echo "Tests have been run!"

ifeq (test,$(firstword $(MAKECMDGOALS)))
TEST_ARGS := $(wordlist 2,$(words $(MAKECMDGOALS)),$(MAKECMDGOALS))
$(eval $(TEST_ARGS):;@:)
endif
test: EXTRA_CFLAGS += -fprofile-arcs -ftest-coverage -fsanitize=address -fsanitize-recover=address -fno-omit-frame-pointer
test: EXTRA_LDFLAGS += -fprofile-arcs -ftest-coverage -fsanitize=address -static-libasan
test: $(BUILDDIR)/apteryxd $(BUILDDIR)/apteryx
@echo "Running apteryx unit tests with gcov and address sanitizer: $<"
@rm -f $(BUILDDIR)/asan-log.*
@rm -f $(BUILDDIR)/*.gcda
$(Q)$(call apteryxd,apteryx -u$(TEST_ARGS))
@echo "Tests have been run!"
@echo "Processing gcov output"
@lcov -q --capture --directory $(BUILDDIR)/ --output-file $(BUILDDIR)/coverage.info
@genhtml -q $(BUILDDIR)/coverage.info --output-directory $(BUILDDIR)/gcov
@echo "GCOV: google-chrome "$(BUILDDIR)"/gcov/index.html"
@cat $(BUILDDIR)/asan-log.* 2>/dev/null | grep -v "False leaks are possible" | grep --color -E "ERROR|Direct leak|SUMMARY|$$" && exit 1 || true

install: all
@install -d $(DESTDIR)/$(PREFIX)/$(LIBDIR)
@install -D libapteryx.so.$(ABI_VERSION) $(DESTDIR)/$(PREFIX)/$(LIBDIR)/
@install -D $(BUILDDIR)/libapteryx.so.$(ABI_VERSION) $(DESTDIR)/$(PREFIX)/$(LIBDIR)/
@ln -sf libapteryx.so.$(ABI_VERSION) $(DESTDIR)/$(PREFIX)/$(LIBDIR)/libapteryx.so
@install -d $(DESTDIR)/$(PREFIX)/include
@install -D apteryx.h $(DESTDIR)/$(PREFIX)/include
@install -d $(DESTDIR)/$(PREFIX)/bin
@install -D apteryxd $(DESTDIR)/$(PREFIX)/bin/
@install -D apteryx $(DESTDIR)/$(PREFIX)/bin/
@install -D $(BUILDDIR)/apteryxd $(DESTDIR)/$(PREFIX)/bin/
@install -D $(BUILDDIR)/apteryx $(DESTDIR)/$(PREFIX)/bin/
@install -d $(DESTDIR)/$(PREFIX)/lib/pkgconfig
@install -D apteryx.pc $(DESTDIR)/$(PREFIX)/lib/pkgconfig/

clean:
@echo "Cleaning..."
@rm -f libapteryx.so* apteryx.so apteryx apteryxd *.o
@rm -fr libapteryx.so* apteryx.so apteryx apteryxd *.o .test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this include/be $BUILDDIR ?


.PHONY: all clean
3 changes: 0 additions & 3 deletions apteryxc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@
#include "internal.h"
#include "apteryx.h"

/* Debug enabled */
bool apteryx_debug = false;

/* Run while true */
static bool running = true;

Expand Down
3 changes: 0 additions & 3 deletions apteryxd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@
#include "apteryx.h"
#include "internal.h"

/* Debug */
bool apteryx_debug = false;

/* Run while true */
static bool running = true;

Expand Down
9 changes: 0 additions & 9 deletions rpc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
struct rpc_instance_s {
/* Protect the instance */
pthread_mutex_t lock;
int pid;

/* General settings */
int timeout;
Expand Down Expand Up @@ -224,7 +223,6 @@ rpc_init (int timeout, rpc_msg_handler handler)

/* Create a new RPC instance */
pthread_mutex_init (&rpc->lock, NULL);
rpc->pid = getpid ();
pthread_sigmask (SIG_SETMASK, NULL, &rpc->worker_sigmask);
rpc->timeout = timeout;
rpc->gc_time = get_time_us ();
Expand Down Expand Up @@ -267,13 +265,6 @@ rpc_shutdown (rpc_instance rpc)

assert (rpc);

/* Check this instance belongs to us */
if (rpc->pid != getpid())
{
ERROR ("RPC: Attempt to shutdown instance (%p) that belongs to pid %d\n", rpc, rpc->pid);
return;
}

DEBUG ("RPC: Shutdown Instance (%p)\n", rpc);

/* Need to wait until all threads are cleaned up */
Expand Down
16 changes: 15 additions & 1 deletion rpc_transport.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,20 @@ rpc_server_parent_get (rpc_server s)
return s->parent;
}

static bool
socket_is_active(socket_info sock)
{
bool active = false;
int sockfd = socket (sock->family, SOCK_STREAM, 0);
if (sockfd >= 0) {
if (connect (sockfd, (struct sockaddr *) &sock->address, sock->address_len) == 0) {
active = true;
}
close (sockfd);
}
return active;
}

bool
rpc_server_die (rpc_server s)
{
Expand All @@ -166,7 +180,7 @@ rpc_server_die (rpc_server s)
}
g_list_free (s->clients);
pthread_mutex_unlock (&s->lock);
if (s->sockinfo->family == AF_UNIX)
if (s->sockinfo->family == AF_UNIX && !socket_is_active (s->sockinfo))
{
unlink (s->sockinfo->address.addr_un.sun_path);
}
Expand Down
11 changes: 10 additions & 1 deletion test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8583,7 +8583,16 @@ run_unit_tests (const char *filter)
CU_TestInfo *test = &suite->pTests[0];
while (test && test->pName)
{
if (all || (filter && strstr (test->pName, filter) != NULL))
if (
#ifdef __SANITIZE_ADDRESS__
/* Do not run performance or fork tests if using the address sanitizer */
(all && !filter && !strstr (test->pName, "perf") &&
!strstr (test->pName, "fork") && !strstr (test->pName, "deadlock")) ||
#else
all ||
#endif
(filter && strstr (test->pName, filter))
)
{
if (CU_add_test(pSuite, test->pName, test->pTestFunc) == NULL)
{
Expand Down