Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Fixed

- HTTP query parameters are now split before URL-decoding, so escaped ampersands in query parameter names and values are preserved correctly (#8024).
- The thread-identifier helpers used by `ccf/ds/logger.h` (`ccf::threading::get_current_thread_id`, `set_current_thread_id`, and `reset_thread_id_generator`) have moved out of `libccf` into a new standalone `ccf_threading` static library, which `find_package(ccf)` exports automatically. This removes a long-standing implicit circular dependency (#7977).
- **Build-graph change for consumers that link CCF component libraries directly.** `ccfcrypto` now links the new `ccf_threading` library, and `ccf_tasks` and `ccf_kv` link `ccf_threading` directly instead of `ccfcrypto`. Downstream targets that linked `ccf_tasks` or `ccf_kv` directly and relied on them transitively supplying CCF cryptography must now link `ccfcrypto` explicitly. Applications built with `add_ccf_app` (which link `ccf` and `ccf_launcher`) are unaffected (#7977).
Comment thread
maxtropets marked this conversation as resolved.

## [7.0.9]

Expand Down
19 changes: 6 additions & 13 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ set(
)

include(${CCF_DIR}/cmake/cose_openssl.cmake)
include(${CCF_DIR}/cmake/threading.cmake)
include(${CCF_DIR}/cmake/crypto.cmake)
include(${CCF_DIR}/cmake/quickjs.cmake)
include(${CCF_DIR}/cmake/evercbor.cmake)
Expand Down Expand Up @@ -299,6 +300,7 @@ add_ccf_static_library(
${CCF_DIR}/src/kv/tx.cpp
${CCF_DIR}/src/kv/untyped_map_handle.cpp
${CCF_DIR}/src/kv/untyped_map_diff.cpp
LINK_LIBS ccf_threading
)

# CCF endpoints lib
Expand Down Expand Up @@ -337,6 +339,7 @@ add_ccf_static_library(
${CCF_DIR}/src/tasks/fan_in_tasks.cpp
${CCF_DIR}/src/tasks/thread_manager.cpp
${CCF_DIR}/src/tasks/worker.cpp
LINK_LIBS ccf_threading
)

find_library(BACKTRACE_LIBRARY backtrace)
Expand Down Expand Up @@ -462,7 +465,6 @@ endif()
set(
CCF_IMPL_SOURCE
${CCF_DIR}/src/enclave/main.cpp
${CCF_DIR}/src/enclave/thread_local.cpp
${CCF_DIR}/src/node/quote.cpp
${CCF_DIR}/src/node/uvm_endorsements.cpp
${CCF_DIR}/src/node/recovery_decision_protocol.cpp
Expand Down Expand Up @@ -973,11 +975,7 @@ if(BUILD_TESTS)
)

# Merkle Tree memory test
add_executable(
merkle_mem
src/node/test/merkle_mem.cpp
${CCF_DIR}/src/enclave/thread_local.cpp
)
add_executable(merkle_mem src/node/test/merkle_mem.cpp)
add_warning_checks(merkle_mem)
target_link_libraries(
merkle_mem
Expand All @@ -988,7 +986,6 @@ if(BUILD_TESTS)
add_executable(
raft_driver
${CMAKE_CURRENT_SOURCE_DIR}/src/consensus/aft/test/driver.cpp
src/enclave/thread_local.cpp
)
add_warning_checks(raft_driver)
enable_coverage(raft_driver)
Expand Down Expand Up @@ -1045,15 +1042,11 @@ if(BUILD_TESTS)
add_picobench(cose_bench SRCS src/crypto/test/cose_bench.cpp LINK_LIBS)
add_picobench(
history_bench
SRCS src/node/test/history_bench.cpp src/enclave/thread_local.cpp
SRCS src/node/test/history_bench.cpp
LINK_LIBS ccf_kv ccf_tasks
)

add_picobench(
kv_bench
SRCS src/kv/test/kv_bench.cpp src/enclave/thread_local.cpp
LINK_LIBS ccf_kv
)
add_picobench(kv_bench SRCS src/kv/test/kv_bench.cpp LINK_LIBS ccf_kv)
add_picobench(merkle_bench SRCS src/node/test/merkle_bench.cpp)
add_picobench(hash_bench SRCS src/ds/test/hash_bench.cpp)

Expand Down
7 changes: 0 additions & 7 deletions cmake/ccf_app.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ function(add_ccf_app name)
target_link_options(${name} PRIVATE LINKER:--no-undefined)
endif()

# Tracked in https://github.com/microsoft/CCF/issues/7596. Workaround for a
# circular dependency between ccf.a and ccfcrypto.a
target_link_options(
${name}
PRIVATE LINKER:--undefined=_ZN3ccf9threading21get_current_thread_idEv
)

set_property(TARGET ${name} PROPERTY POSITION_INDEPENDENT_CODE ON)

add_san(${name})
Expand Down
12 changes: 4 additions & 8 deletions cmake/common.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ endfunction()

# Unit test wrapper
function(add_unit_test name)
add_executable(${name} ${CCF_DIR}/src/enclave/thread_local.cpp ${ARGN})
add_executable(${name} ${ARGN})
target_include_directories(
${name}
PRIVATE src ${CCFCRYPTO_INC} ${CCF_DIR}/3rdparty/test
Expand All @@ -54,7 +54,7 @@ endfunction()

# Fuzz test wrapper (requires -DFUZZING=ON)
function(add_fuzz_test name)
add_executable(${name} ${CCF_DIR}/src/enclave/thread_local.cpp ${ARGN})
add_executable(${name} ${ARGN})
target_compile_options(${name} PRIVATE -fsanitize=fuzzer)
target_link_options(${name} PRIVATE -fsanitize=fuzzer)
target_include_directories(${name} PRIVATE src ${CCFCRYPTO_INC})
Expand Down Expand Up @@ -82,7 +82,7 @@ endfunction()

# Test binary wrapper
function(add_test_bin name)
add_executable(${name} ${CCF_DIR}/src/enclave/thread_local.cpp ${ARGN})
add_executable(${name} ${ARGN})
target_include_directories(
${name}
PRIVATE src ${CCFCRYPTO_INC} ${CCF_DIR}/3rdparty/test
Expand Down Expand Up @@ -256,11 +256,7 @@ function(add_picobench name)
"SRCS;INCLUDE_DIRS;LINK_LIBS"
)

add_executable(
${name}
${PARSED_ARGS_SRCS}
${CCF_DIR}/src/enclave/thread_local.cpp
)
add_executable(${name} ${PARSED_ARGS_SRCS})

target_include_directories(${name} PRIVATE src ${PARSED_ARGS_INCLUDE_DIRS})

Expand Down
2 changes: 1 addition & 1 deletion cmake/crypto.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ add_san(ccfcrypto)
add_hardening(ccfcrypto)
add_tidy(ccfcrypto)

target_link_libraries(ccfcrypto PUBLIC crypto ssl evercbor)
target_link_libraries(ccfcrypto PUBLIC crypto ssl evercbor ccf_threading)
target_link_libraries(
ccfcrypto
PUBLIC
Expand Down
12 changes: 12 additions & 0 deletions cmake/threading.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the Apache 2.0 License.

# Minimal foundational library owning the thread-id support that the logger
# headers depend on (ccf::threading::get_current_thread_id and friends).
#
# Keeping this in its own library lets every logger user resolve the symbol by
Comment thread
maxtropets marked this conversation as resolved.
# depending on ccf_threading, without having to link libccf.
add_ccf_static_library(
ccf_threading
SRCS ${CCF_DIR}/src/threading/thread_ids.cpp
)
2 changes: 1 addition & 1 deletion include/ccf/threading/thread_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ namespace ccf::threading
uint16_t get_current_thread_id();
void set_current_thread_id(ThreadID to);
void reset_thread_id_generator(ThreadID to = MAIN_THREAD_ID);
}
}
23 changes: 23 additions & 0 deletions src/ds/test/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,29 @@

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>
#include <thread>

TEST_CASE("Thread IDs are provided by the logger headers")
{
ccf::threading::reset_thread_id_generator();
ccf::threading::set_current_thread_id(ccf::threading::MAIN_THREAD_ID);

REQUIRE(
ccf::threading::get_current_thread_id() == ccf::threading::MAIN_THREAD_ID);

ccf::threading::set_current_thread_id(42);
REQUIRE(ccf::threading::get_current_thread_id() == 42);

ccf::threading::reset_thread_id_generator(7);
ccf::threading::ThreadID thread_id = ccf::threading::invalid_thread_id;
std::thread t(
[&thread_id] { thread_id = ccf::threading::get_current_thread_id(); });
t.join();
REQUIRE(thread_id == 7);

ccf::threading::reset_thread_id_generator();
ccf::threading::set_current_thread_id(ccf::threading::MAIN_THREAD_ID);
}

template <typename Base>
class TestLogger : public Base
Expand Down
14 changes: 8 additions & 6 deletions src/enclave/thread_local.cpp → src/threading/thread_ids.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@
// Licensed under the Apache 2.0 License.
#include "ccf/threading/thread_ids.h"

#include <atomic>

namespace ccf::threading
{
namespace
{
std::atomic<ThreadID> next_thread_id = MAIN_THREAD_ID;
}

uint16_t& current_thread_id()
{
thread_local ThreadID this_thread_id = next_thread_id.fetch_add(1);
return this_thread_id;
ThreadID& current_thread_id()
{
thread_local ThreadID this_thread_id = next_thread_id.fetch_add(1);
return this_thread_id;
}
}

uint16_t get_current_thread_id()
Expand All @@ -29,4 +31,4 @@ namespace ccf::threading
{
next_thread_id.store(to);
}
}
}
1 change: 0 additions & 1 deletion tests/perf-system/submitter/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ add_executable(
${SUBMITTER_DIR}/submit.cpp
${SUBMITTER_DIR}/handle_arguments.h
${SUBMITTER_DIR}/parquet_data.h
${CCF_DIR}/src/enclave/thread_local.cpp
)

add_warning_checks(submit)
Expand Down