Fix memory leak with sidecar trace sender

bwoebi · bwoebi · commit b9fa0f0c5617 · 2024-10-04T18:38:31.000+02:00
And set LSAN_OPTIONS=fast_unwind_on_malloc=0 for more complete traces

Signed-off-by: Bob Weinand &lt;bob.weinand@datadoghq.com&gt;
diff --git a/Makefile b/Makefile
@@ -1,4 +1,5 @@
 Q := @
+, := ,
 PROJECT_ROOT := ${PWD}
 TRACER_SOURCE_DIR := $(PROJECT_ROOT)/src/
 ASAN ?= $(shell ldd $(shell which php) 2>/dev/null | grep -q libasan && echo 1)
@@ -156,7 +157,7 @@ run_tests: $(TEST_FILES) $(TEST_STUB_FILES) $(BUILD_DIR)/run-tests.php
 	DD_TRACE_GIT_METADATA_ENABLED=0 $(RUN_TESTS_CMD) $(TESTS)
 
 test_c: $(SO_FILE) $(TEST_FILES) $(TEST_STUB_FILES) $(BUILD_DIR)/run-tests.php
-	$(if $(ASAN), USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1) DD_TRACE_GIT_METADATA_ENABLED=0 $(RUN_TESTS_CMD) -d extension=$(SO_FILE) $(BUILD_DIR)/$(subst $(BUILD_DIR_NAME)/,,$(TESTS))
+	$(if $(ASAN), USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1 LSAN_OPTIONS=fast_unwind_on_malloc=0$${LSAN_OPTIONS:+$(,)$${LSAN_OPTIONS}}) DD_TRACE_GIT_METADATA_ENABLED=0 $(RUN_TESTS_CMD) -d extension=$(SO_FILE) $(BUILD_DIR)/$(subst $(BUILD_DIR_NAME)/,,$(TESTS))
 
 test_c_coverage: dist_clean
 	DD_TRACE_DOCKER_DEBUG=1 EXTRA_CFLAGS="-fprofile-arcs -ftest-coverage" $(MAKE) test_c || exit 0
@@ -1405,7 +1406,7 @@ test_api_unit: composer.lock global_test_run_dependencies
 
 # Just test it does not crash, i.e. the exit code
 test_internal_api_randomized: $(SO_FILE)
-	$(if $(ASAN), USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1) php -n -ddatadog.trace.cli_enabled=1 -d extension=$(SO_FILE) tests/internal-api-stress-test.php 2> >(grep -A 1000 ==============)
+	$(if $(ASAN), DD_TRACE_DEBUG=1 USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1 LSAN_OPTIONS=fast_unwind_on_malloc=0$${LSAN_OPTIONS:+$(,)$${LSAN_OPTIONS}}) php -n -ddatadog.trace.cli_enabled=1 -d extension=$(SO_FILE) tests/internal-api-stress-test.php 2> >(grep -A 1000 ==============)
 
 composer.lock: composer.json
 	$(call run_composer_with_retry,,)
diff --git a/ext/ddtrace.c b/ext/ddtrace.c
@@ -2460,7 +2460,7 @@ PHP_FUNCTION(dd_trace_internal_fn) {
             } else
 #endif
             if (ddtrace_sidecar) {
-                ddog_sidecar_flush_traces(&ddtrace_sidecar);
+                ddtrace_ffi_try("Failed synchronously flushing traces", ddog_sidecar_flush_traces(&ddtrace_sidecar));
             }
             RETVAL_TRUE;
 #ifndef _WIN32
@@ -2577,7 +2577,7 @@ PHP_FUNCTION(dd_trace_synchronous_flush) {
     } else
 #endif
     if (ddtrace_sidecar) {
-        ddog_sidecar_flush_traces(&ddtrace_sidecar);
+        ddtrace_ffi_try("Failed synchronously flushing traces", ddog_sidecar_flush_traces(&ddtrace_sidecar));
     }
     RETURN_NULL();
 }
diff --git a/ext/sidecar.c b/ext/sidecar.c
@@ -354,7 +354,7 @@ void ddtrace_sidecar_submit_root_span_data_direct(ddtrace_root_span_data *root)
         changed = ddog_remote_configs_service_env_change(DDTRACE_G(remote_config_state), service_slice, env_slice, version_slice);
     }
     if (changed || !root) {
-        ddog_sidecar_set_remote_config_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), service_slice, env_slice, version_slice, &DDTRACE_G(active_global_tags));
+        ddtrace_ffi_try("Failed sending remote config data", ddog_sidecar_set_remote_config_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), service_slice, env_slice, version_slice, &DDTRACE_G(active_global_tags)));
     }
 
     if (free_string) {
diff --git a/ext/sidecar.h b/ext/sidecar.h
@@ -46,7 +46,7 @@ static inline zend_string *dd_CharSlice_to_zend_string(ddog_CharSlice slice) {
 static inline bool ddtrace_ffi_try(const char *msg, ddog_MaybeError maybe_error) {
     if (maybe_error.tag == DDOG_OPTION_ERROR_SOME_ERROR) {
         ddog_CharSlice error = ddog_Error_message(&maybe_error.some);
-        LOG(ERROR, "%s: %.*s", msg, (int) error.len, error.ptr);
+        LOG(WARN, "%s: %.*s", msg, (int) error.len, error.ptr);
         ddog_MaybeError_drop(maybe_error);
         return false;
     }

Original file line number	Diff line number	Diff line change
`@@ -2460,7 +2460,7 @@ PHP_FUNCTION(dd_trace_internal_fn) {`
`2460`	`2460`	`} else`
`2461`	`2461`	`#endif`
`2462`	`2462`	`if (ddtrace_sidecar) {`
`2463`		`- ddog_sidecar_flush_traces(&ddtrace_sidecar);`
	`2463`	`+ ddtrace_ffi_try("Failed synchronously flushing traces", ddog_sidecar_flush_traces(&ddtrace_sidecar));`
`2464`	`2464`	`}`
`2465`	`2465`	`RETVAL_TRUE;`
`2466`	`2466`	`#ifndef _WIN32`
`@@ -2577,7 +2577,7 @@ PHP_FUNCTION(dd_trace_synchronous_flush) {`
`2577`	`2577`	`} else`
`2578`	`2578`	`#endif`
`2579`	`2579`	`if (ddtrace_sidecar) {`
`2580`		`- ddog_sidecar_flush_traces(&ddtrace_sidecar);`
	`2580`	`+ ddtrace_ffi_try("Failed synchronously flushing traces", ddog_sidecar_flush_traces(&ddtrace_sidecar));`
`2581`	`2581`	`}`
`2582`	`2582`	`RETURN_NULL();`
`2583`	`2583`	`}`
Original file line number	Diff line number	Diff line change
`@@ -354,7 +354,7 @@ void ddtrace_sidecar_submit_root_span_data_direct(ddtrace_root_span_data *root)`
`354`	`354`	`changed = ddog_remote_configs_service_env_change(DDTRACE_G(remote_config_state), service_slice, env_slice, version_slice);`
`355`	`355`	`}`
`356`	`356`	`if (changed \|\| !root) {`
`357`		`- ddog_sidecar_set_remote_config_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), service_slice, env_slice, version_slice, &DDTRACE_G(active_global_tags));`
	`357`	`+ ddtrace_ffi_try("Failed sending remote config data", ddog_sidecar_set_remote_config_data(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), service_slice, env_slice, version_slice, &DDTRACE_G(active_global_tags)));`
`358`	`358`	`}`
`359`	`359`
`360`	`360`	`if (free_string) {`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ static inline zend_string *dd_CharSlice_to_zend_string(ddog_CharSlice slice) {`
`46`	`46`	`static inline bool ddtrace_ffi_try(const char *msg, ddog_MaybeError maybe_error) {`
`47`	`47`	`if (maybe_error.tag == DDOG_OPTION_ERROR_SOME_ERROR) {`
`48`	`48`	`ddog_CharSlice error = ddog_Error_message(&maybe_error.some);`
`49`		`- LOG(ERROR, "%s: %.*s", msg, (int) error.len, error.ptr);`
	`49`	`+ LOG(WARN, "%s: %.*s", msg, (int) error.len, error.ptr);`
`50`	`50`	`ddog_MaybeError_drop(maybe_error);`
`51`	`51`	`return false;`
`52`	`52`	`}`