From 2085cbb45161b78712c3214add56edbb26aa7713 Mon Sep 17 00:00:00 2001 From: Richard Wheeler Date: Mon, 15 Jun 2026 08:34:53 -0400 Subject: [PATCH] net.mbedtls: enable MBEDTLS_THREADING_C on Windows and macOS (#27437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * net.mbedtls: enable MBEDTLS_THREADING_C on Windows and macOS mbedtls' internal locking (around the shared RNG, RSA key blinding and the library globals) was enabled only on Linux/FreeBSD/OpenBSD and disabled on Windows and macOS, because V only wired up mbedtls' pthread mutex backend and Windows (MSVC) has no pthreads. This blocks any design that shares an mbedtls config/RNG across threads (e.g. parallel TLS server handshakes). - macOS: it has pthreads, so add it to the gate (it was simply omitted). - Windows: enable MBEDTLS_THREADING_ALT and supply the four mutex callbacks over a Win32 CRITICAL_SECTION (vlib/net/mbedtls/ mbedtls_threading.h + thirdparty threading_alt.h), installed once via mbedtls_threading_set_alt() from the net.mbedtls module init() before any TLS use. The setup is a no-op on non-Windows builds. Folded into mbedtls.patch so it survives the next mbedtls upgrade. Note: changing the threading config alters mbedtls struct layouts, so an existing local build must rebuild the mbedtls objects (touch the sources or delete the cached .o); fresh/CI builds are unaffected. Verified on Windows: net.http TLS handshakes work with threading enabled (the mutex callbacks are exercised on every TLS operation); the only server_tls_test failures are the pre-existing net.shutdown idle-interrupt ones, unchanged from master. macOS to be verified on Apple Silicon. Closes #27436 Co-Authored-By: Claude Opus 4.8 * builder: invalidate thirdparty objects on any module header change Cached thirdparty objects were only invalidated against the source `.c` and its sibling headers in the source directory (e.g. thirdparty/mbedtls/library/). A change to a config header elsewhere in the module — notably thirdparty/mbedtls/include/mbedtls/mbedtls_config.h, which alters mbedtls struct layouts — left stale objects cached, so an existing checkout silently reused the old layout (link errors on MSVC, missing locking on macOS) until `v wipe-cache` was run by hand. Worse, the MSVC path reused a cached `.obj` whenever it merely existed, with no mtime comparison against the source or any header at all — the root of the long-standing "-cc msvc: delete library/*.obj by hand" workaround. Both paths now share one decision via thirdparty_deps_mtime(), which folds the newest `.h`/`.hpp` mtime under the whole thirdparty module root (include/ included) into the cache key. The recursive header scan is memoized per module root, so it runs once per module rather than once per object. Source-less bundled objects keep their reuse-if-present behavior. Verified (tcc): editing include/mbedtls/mbedtls_config.h now rebuilds the mbedtls objects, an unchanged tree rebuilds nothing, and the build re-stabilizes afterwards. Co-Authored-By: WOZCODE * builder: test thirdparty header invalidation spans the module include/ tree Regression guard for the cache-invalidation fix: thirdparty objects must rebuild when any header in the module changes, including config headers under include/ (e.g. mbedtls_config.h), not just siblings of the .c. - test_thirdparty_module_root_spans_library_and_include: the pure path helper resolves a library/ source to the whole thirdparty/ root, normalizes separators, and falls back to the source dir off-tree. - test_thirdparty_deps_mtime_includes_module_include_headers: a temp thirdparty//{library,include} tree where an include/ header is newer than the .c; thirdparty_deps_mtime must reflect it. Verified to fail against the old source-dir-only scan. This helper is shared by the C and MSVC object paths, so it guards both. Co-Authored-By: WOZCODE --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: WOZCODE --- .../mbedtls/include/mbedtls/mbedtls_config.h | 8 ++- .../mbedtls/include/mbedtls/threading_alt.h | 20 ++++++ thirdparty/mbedtls/mbedtls.patch | 34 +++++++++- vlib/net/mbedtls/mbedtls.c.v | 14 ++++ vlib/net/mbedtls/mbedtls_threading.h | 62 +++++++++++++++++ vlib/v/builder/builder.v | 4 ++ vlib/v/builder/cc.v | 68 ++++++++++++------- vlib/v/builder/cc_test.v | 49 +++++++++++++ vlib/v/builder/msvc_windows.v | 24 +++++-- 9 files changed, 249 insertions(+), 34 deletions(-) create mode 100644 thirdparty/mbedtls/include/mbedtls/threading_alt.h create mode 100644 vlib/net/mbedtls/mbedtls_threading.h diff --git a/thirdparty/mbedtls/include/mbedtls/mbedtls_config.h b/thirdparty/mbedtls/include/mbedtls/mbedtls_config.h index dc98200a2..e20fc0a6c 100644 --- a/thirdparty/mbedtls/include/mbedtls/mbedtls_config.h +++ b/thirdparty/mbedtls/include/mbedtls/mbedtls_config.h @@ -4457,9 +4457,15 @@ #define MBEDTLS_PADLOCK_C #endif // __TINYC__ -#if ( defined(__linux__) || defined(__FreeBSD__) ) || defined (__OpenBSD__) +#if ( defined(__linux__) || defined(__FreeBSD__) ) || defined (__OpenBSD__) || defined(__APPLE__) #define MBEDTLS_THREADING_PTHREAD #define MBEDTLS_THREADING_C +#elif defined(_WIN32) +// Windows has no pthreads; the mutex callbacks are provided via +// MBEDTLS_THREADING_ALT (see vlib/net/mbedtls/mbedtls_threading.h, installed by +// mbedtls_threading_set_alt() from the net.mbedtls module init()). +#define MBEDTLS_THREADING_ALT +#define MBEDTLS_THREADING_C #else #undef MBEDTLS_THREADING_PTHREAD #undef MBEDTLS_THREADING_C diff --git a/thirdparty/mbedtls/include/mbedtls/threading_alt.h b/thirdparty/mbedtls/include/mbedtls/threading_alt.h new file mode 100644 index 000000000..b06f8536d --- /dev/null +++ b/thirdparty/mbedtls/include/mbedtls/threading_alt.h @@ -0,0 +1,20 @@ +/* + * threading_alt.h - mbedtls_threading_mutex_t for MBEDTLS_THREADING_ALT. + * + * V provides this on Windows (which has no pthreads) so that + * MBEDTLS_THREADING_C can be enabled. The mutex is a Win32 CRITICAL_SECTION; + * the callbacks that operate on it live in + * vlib/net/mbedtls/mbedtls_threading.h and are installed once at startup via + * mbedtls_threading_set_alt() from the net.mbedtls module init(). + */ +#ifndef MBEDTLS_THREADING_ALT_H +#define MBEDTLS_THREADING_ALT_H + +#include + +typedef struct mbedtls_threading_mutex_t { + CRITICAL_SECTION cs; + char is_valid; +} mbedtls_threading_mutex_t; + +#endif /* MBEDTLS_THREADING_ALT_H */ diff --git a/thirdparty/mbedtls/mbedtls.patch b/thirdparty/mbedtls/mbedtls.patch index ddd2506ea..21eaea72c 100644 --- a/thirdparty/mbedtls/mbedtls.patch +++ b/thirdparty/mbedtls/mbedtls.patch @@ -18,7 +18,7 @@ diff -ur mbedtls.orig/include/mbedtls/check_config.h mbedtls/include/mbedtls/che diff -ur mbedtls.orig/include/mbedtls/mbedtls_config.h mbedtls/include/mbedtls/mbedtls_config.h --- mbedtls.orig/include/mbedtls/mbedtls_config.h 2026-04-02 17:26:31.910276365 +0200 +++ mbedtls/include/mbedtls/mbedtls_config.h 2026-04-02 17:27:29.374384448 +0200 -@@ -4435,3 +4435,22 @@ +@@ -4435,3 +4435,28 @@ //#define MBEDTLS_X509_MAX_FILE_PATH_LEN 512 /**< Maximum length of a path/filename string in bytes including the null terminator character ('\0'). */ /** \} name SECTION: Module configuration options */ @@ -34,13 +34,43 @@ diff -ur mbedtls.orig/include/mbedtls/mbedtls_config.h mbedtls/include/mbedtls/m +#define MBEDTLS_PADLOCK_C +#endif // __TINYC__ + -+#if ( defined(__linux__) || defined(__FreeBSD__) ) || defined (__OpenBSD__) ++#if ( defined(__linux__) || defined(__FreeBSD__) ) || defined (__OpenBSD__) || defined(__APPLE__) +#define MBEDTLS_THREADING_PTHREAD +#define MBEDTLS_THREADING_C ++#elif defined(_WIN32) ++// Windows has no pthreads; the mutex callbacks are provided via ++// MBEDTLS_THREADING_ALT (see vlib/net/mbedtls/mbedtls_threading.h, installed by ++// mbedtls_threading_set_alt() from the net.mbedtls module init()). ++#define MBEDTLS_THREADING_ALT ++#define MBEDTLS_THREADING_C +#else +#undef MBEDTLS_THREADING_PTHREAD +#undef MBEDTLS_THREADING_C +#endif +diff -ur mbedtls.orig/include/mbedtls/threading_alt.h mbedtls/include/mbedtls/threading_alt.h +--- mbedtls.orig/include/mbedtls/threading_alt.h 1970-01-01 00:00:00.000000000 +0000 ++++ mbedtls/include/mbedtls/threading_alt.h 2026-06-12 00:00:00.000000000 +0000 +@@ -0,0 +1,20 @@ ++/* ++ * threading_alt.h - mbedtls_threading_mutex_t for MBEDTLS_THREADING_ALT. ++ * ++ * V provides this on Windows (which has no pthreads) so that ++ * MBEDTLS_THREADING_C can be enabled. The mutex is a Win32 CRITICAL_SECTION; ++ * the callbacks that operate on it live in ++ * vlib/net/mbedtls/mbedtls_threading.h and are installed once at startup via ++ * mbedtls_threading_set_alt() from the net.mbedtls module init(). ++ */ ++#ifndef MBEDTLS_THREADING_ALT_H ++#define MBEDTLS_THREADING_ALT_H ++ ++#include ++ ++typedef struct mbedtls_threading_mutex_t { ++ CRITICAL_SECTION cs; ++ char is_valid; ++} mbedtls_threading_mutex_t; ++ ++#endif /* MBEDTLS_THREADING_ALT_H */ diff -ur mbedtls.orig/library/alignment.h mbedtls/library/alignment.h --- mbedtls.orig/library/alignment.h 2026-04-02 17:26:31.918276380 +0200 +++ mbedtls/library/alignment.h 2026-04-02 17:30:51.350689732 +0200 diff --git a/vlib/net/mbedtls/mbedtls.c.v b/vlib/net/mbedtls/mbedtls.c.v index 2bb2ba3ce..6647327a8 100644 --- a/vlib/net/mbedtls/mbedtls.c.v +++ b/vlib/net/mbedtls/mbedtls.c.v @@ -135,7 +135,21 @@ $if prod && opt_size ? { #include #include #include +#include #insert "@VEXEROOT/vlib/net/mbedtls/mbedtls_helpers.h" +#insert "@VEXEROOT/vlib/net/mbedtls/mbedtls_threading.h" + +// v_mbedtls_threading_setup installs the mutex callbacks mbedtls needs when it +// is built with MBEDTLS_THREADING_ALT (Windows). On platforms that use pthread +// threading or no threading it is a no-op. Defined in mbedtls_threading.h. +fn C.v_mbedtls_threading_setup() + +// init installs mbedtls' thread-safety callbacks once, before any TLS use, so +// the library's shared state (RNG, key blinding, internal globals) is safe to +// use across threads. A no-op on non-Windows builds. +fn init() { + C.v_mbedtls_threading_setup() +} @[typedef] pub struct C.mbedtls_net_context { diff --git a/vlib/net/mbedtls/mbedtls_threading.h b/vlib/net/mbedtls/mbedtls_threading.h new file mode 100644 index 000000000..a690842e2 --- /dev/null +++ b/vlib/net/mbedtls/mbedtls_threading.h @@ -0,0 +1,62 @@ +/* + * mbedtls_threading.h - Win32 mutex callbacks for MBEDTLS_THREADING_ALT. + * + * On Windows, mbedtls is built with MBEDTLS_THREADING_C + MBEDTLS_THREADING_ALT + * (see thirdparty/mbedtls/include/mbedtls/mbedtls_config.h). The library then + * expects the embedder to supply the four mutex primitives via + * mbedtls_threading_set_alt(). These wrap a Win32 CRITICAL_SECTION; the matching + * mbedtls_threading_mutex_t type is defined in + * thirdparty/mbedtls/include/mbedtls/threading_alt.h. + * + * v_mbedtls_threading_setup() is called once from net.mbedtls' init() before any + * TLS use. On every other platform it is a no-op (pthread threading, or none). + */ +#ifndef V_MBEDTLS_THREADING_H +#define V_MBEDTLS_THREADING_H + +#if defined(_WIN32) && defined(MBEDTLS_THREADING_ALT) + +#include +#include +#include + +static void v_mbedtls_mutex_init(mbedtls_threading_mutex_t *m) { + InitializeCriticalSection(&m->cs); + m->is_valid = 1; +} + +static void v_mbedtls_mutex_free(mbedtls_threading_mutex_t *m) { + if (m->is_valid) { + DeleteCriticalSection(&m->cs); + m->is_valid = 0; + } +} + +static int v_mbedtls_mutex_lock(mbedtls_threading_mutex_t *m) { + if (!m->is_valid) { + return MBEDTLS_ERR_THREADING_BAD_INPUT_DATA; + } + EnterCriticalSection(&m->cs); + return 0; +} + +static int v_mbedtls_mutex_unlock(mbedtls_threading_mutex_t *m) { + if (!m->is_valid) { + return MBEDTLS_ERR_THREADING_BAD_INPUT_DATA; + } + LeaveCriticalSection(&m->cs); + return 0; +} + +static void v_mbedtls_threading_setup(void) { + mbedtls_threading_set_alt(v_mbedtls_mutex_init, v_mbedtls_mutex_free, + v_mbedtls_mutex_lock, v_mbedtls_mutex_unlock); +} + +#else /* not (Windows + THREADING_ALT) */ + +static void v_mbedtls_threading_setup(void) { } + +#endif + +#endif /* V_MBEDTLS_THREADING_H */ diff --git a/vlib/v/builder/builder.v b/vlib/v/builder/builder.v index bf7fb8043..724334026 100644 --- a/vlib/v/builder/builder.v +++ b/vlib/v/builder/builder.v @@ -59,6 +59,10 @@ pub mut: str_args string // for parallel_cc mode only, to know which cc args to use (like -I etc) last_cc_cmd string // the most recently executed C compiler command; reused to regenerate a #line annotated report disable_flto bool + // thirdparty_header_mtimes memoizes the newest header mtime under a + // thirdparty module root (see thirdparty_deps_mtime), so the recursive scan + // runs once per module instead of once per compiled object. + thirdparty_header_mtimes map[string]i64 } struct CFunctionCallCollector { diff --git a/vlib/v/builder/cc.v b/vlib/v/builder/cc.v index b10f83a2d..97e3c0387 100644 --- a/vlib/v/builder/cc.v +++ b/vlib/v/builder/cc.v @@ -2453,28 +2453,50 @@ fn (v &Builder) should_compile_bundled_thirdparty_object_from_source(obj_path st return v.ccoptions.cc == .tcc && v.pref.os == .macos } -// latest_thirdparty_header_mtime returns the most recent mtime among `.h` and -// `.hpp` files in the directory of `source_file`. It is used as an extra cache -// key for third-party object compilation, so that header-only patches (for -// example to mbedtls/library/alignment.h) reliably invalidate stale `.o` -// files that were produced before the header change. -fn latest_thirdparty_header_mtime(source_file string) i64 { +// thirdparty_module_root returns the `thirdparty/` directory that +// `source_file` belongs to, or its own directory when the path is not under a +// `thirdparty/` tree (so the scan degrades to the old source-dir-only behavior). +fn thirdparty_module_root(source_file string) string { + norm := source_file.replace('\\', '/') + marker := '/thirdparty/' + idx := norm.index(marker) or { return os.dir(source_file) } + rest := norm[idx + marker.len..] + mod_name := rest.all_before('/') + if mod_name == '' || mod_name == rest { + // No further path component after the module name: fall back. + return os.dir(source_file) + } + return norm[..idx] + marker + mod_name +} + +// thirdparty_deps_mtime returns the newest mtime among `source_file` itself and +// every `.h`/`.hpp` header under its thirdparty module root. It is the shared +// cache key for third-party object compilation (both the C and MSVC paths), so +// that header-only edits anywhere in the module — including config headers under +// `include/`, e.g. mbedtls/include/mbedtls/mbedtls_config.h, not just siblings +// of the `.c` — reliably invalidate stale objects built before the change. The +// recursive header scan is memoized per module root. +fn (mut v Builder) thirdparty_deps_mtime(source_file string) i64 { if source_file == '' { return 0 } - dir := os.dir(source_file) - entries := os.ls(dir) or { return 0 } - mut latest := i64(0) - for f in entries { - if !(f.ends_with('.h') || f.ends_with('.hpp')) { - continue - } - m := os.file_last_mod_unix(os.join_path(dir, f)) - if m > latest { - latest = m + src_mtime := os.file_last_mod_unix(source_file) + root := thirdparty_module_root(source_file) + hdr_mtime := v.thirdparty_header_mtimes[root] or { + mut latest := i64(0) + for f in os.walk_ext(root, '') { + if !(f.ends_with('.h') || f.ends_with('.hpp')) { + continue + } + m := os.file_last_mod_unix(f) + if m > latest { + latest = m + } } + v.thirdparty_header_mtimes[root] = latest + latest } - return latest + return if hdr_mtime > src_mtime { hdr_mtime } else { src_mtime } } fn (mut v Builder) build_thirdparty_obj_file(mod string, path string, moduleflags []cflag.CFlag) { @@ -2525,14 +2547,12 @@ fn (mut v Builder) build_thirdparty_obj_file(mod string, path string, moduleflag } if os.exists(opath) { opath_mtime := os.file_last_mod_unix(opath) - src_mtime := os.file_last_mod_unix(source_file) - // Header-only edits in the source directory (e.g. mbedtls's - // alignment.h) leave every sibling `.c` untouched, so a pure + // Header-only edits (e.g. mbedtls's alignment.h, or a config header + // under include/) leave every `.c` untouched, so a pure // `opath_mtime < src_mtime` test would silently reuse stale objects - // that still reference the old headers. Treat the newest sibling - // header as part of the cache key as well. - hdr_mtime := latest_thirdparty_header_mtime(source_file) - deps_mtime := if hdr_mtime > src_mtime { hdr_mtime } else { src_mtime } + // that still reference the old headers. Fold the newest module header + // into the cache key as well. + deps_mtime := v.thirdparty_deps_mtime(source_file) if compile_bundled_source && !cached_object_was_built_from_source { rebuild_reason_message = '${os.quoted_path(opath)} was copied from a bundled object, rebuilding it from ${os.quoted_path(source_file)} ...' } else if opath_mtime < deps_mtime { diff --git a/vlib/v/builder/cc_test.v b/vlib/v/builder/cc_test.v index c9ffceefd..cc8b6659a 100644 --- a/vlib/v/builder/cc_test.v +++ b/vlib/v/builder/cc_test.v @@ -1,6 +1,7 @@ module builder import os +import time import v.pref fn test_ccompiler_is_available_with_existing_absolute_path() { @@ -800,6 +801,54 @@ fn test_c_error_should_send_bug_report_for_regular_c_error() { assert c_error_should_send_bug_report(c_output) } +fn test_thirdparty_module_root_spans_library_and_include() { + // A source under library/ resolves to the whole `thirdparty/` root, so a + // sibling include/ tree (e.g. mbedtls config headers) is in scope when + // computing the object's header-dependency mtime. + assert thirdparty_module_root('/x/y/thirdparty/mbedtls/library/aes.c') == '/x/y/thirdparty/mbedtls' + // Windows-style separators are normalized to forward slashes. + assert thirdparty_module_root('S:\\repo\\thirdparty\\mbedtls\\include\\mbedtls\\cfg.h') == 'S:/repo/thirdparty/mbedtls' + // A path that is not under a thirdparty/ tree falls back to the source dir. + assert thirdparty_module_root('/tmp/foo/bar.c') == os.dir('/tmp/foo/bar.c') +} + +// Regression guard for vlang/v#27437: thirdparty objects must rebuild when a +// header anywhere in the module changes, including config headers under +// include/ (not just siblings of the .c). thirdparty_deps_mtime is the shared +// cache key for both the C and MSVC object paths, so this protects both. +fn test_thirdparty_deps_mtime_includes_module_include_headers() { + test_root := os.join_path(os.vtmp_dir(), 'v_builder_thirdparty_deps_${os.getpid()}') + os.rmdir_all(test_root) or {} + defer { + os.rmdir_all(test_root) or {} + } + lib_dir := os.join_path(test_root, 'thirdparty', 'mymod', 'library') + inc_dir := os.join_path(test_root, 'thirdparty', 'mymod', 'include', 'mymod') + os.mkdir_all(lib_dir) or { panic(err) } + os.mkdir_all(inc_dir) or { panic(err) } + source := os.join_path(lib_dir, 'foo.c') + sibling := os.join_path(lib_dir, 'foo.h') + os.write_file(source, 'int foo(void){return 0;}') or { panic(err) } + os.write_file(sibling, '/* sibling header */') or { panic(err) } + // file_last_mod_unix is second-resolution, so a >1s gap is needed for the + // config header to be observably newer than the source. + time.sleep(1100 * time.millisecond) + config := os.join_path(inc_dir, 'mymod_config.h') + os.write_file(config, '#define MYMOD 1') or { panic(err) } + + mut b := new_builder_for_args([hello_world_example()]) + deps := b.thirdparty_deps_mtime(source) + config_mtime := os.file_last_mod_unix(config) + source_mtime := os.file_last_mod_unix(source) + assert config_mtime > source_mtime, 'precondition: the include/ config header must be newer than the source' + // The config header lives under include/, not in the source directory, yet + // the dependency mtime must still reflect it — otherwise stale objects are + // reused after a config-header change (the bug this fix addresses). + assert deps == config_mtime, 'thirdparty_deps_mtime must fold in include/ headers; got ${deps}, want ${config_mtime}' + // Memoized per module root: a second call returns the same value. + assert b.thirdparty_deps_mtime(source) == config_mtime +} + fn prepare_test_ccompiler_alias(test_root string, compiler_name string, version_output string) string { os.rmdir_all(test_root) or {} os.mkdir_all(test_root) or { panic(err) } diff --git a/vlib/v/builder/msvc_windows.v b/vlib/v/builder/msvc_windows.v index 720546110..205cbb7c4 100644 --- a/vlib/v/builder/msvc_windows.v +++ b/vlib/v/builder/msvc_windows.v @@ -442,18 +442,28 @@ fn (mut v Builder) build_thirdparty_obj_file_with_msvc(mod string, path string, trace_thirdparty_obj_files := 'trace_thirdparty_obj_files' in v.pref.compile_defines path_without_o_postfix := path.all_before_last('.') obj_path := v.msvc_thirdparty_obj_path(mod, path, '') - if os.exists(obj_path) { - // println('${obj_path} already built.') - return - } - if trace_thirdparty_obj_files { - println('${obj_path} not found, building it (with msvc)...') - } cfile := if os.exists('${path_without_o_postfix}.c') { '${path_without_o_postfix}.c' } else { '${path_without_o_postfix}.cpp' } + if os.exists(obj_path) { + // Reuse the cached object only when it is newer than its source and + // every header in the thirdparty module (config headers under include/ + // included; see thirdparty_deps_mtime). Without this, a config/header + // change silently keeps a stale `.obj` — the long-standing `-cc msvc` + // "delete library/*.obj by hand" footgun. Source-less bundled objects + // (no sibling .c/.cpp) keep the previous reuse-if-present behavior. + src_exists := os.exists('${path_without_o_postfix}.c') + || os.exists('${path_without_o_postfix}.cpp') + if !src_exists || os.file_last_mod_unix(obj_path) >= v.thirdparty_deps_mtime(cfile) { + // println('${obj_path} already built.') + return + } + } + if trace_thirdparty_obj_files { + println('${obj_path} not found or stale, building it (with msvc)...') + } flags := v.msvc_string_flags(moduleflags) inc_dirs := flags.inc_paths.join(' ') defines := flags.defines.join(' ') -- 2.39.5