diff --git a/AGENTS.md b/AGENTS.md index c39f3177..ca805bc7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ Apple Clang release on macOS. C++23 support is required in both the compiler and its standard library. `configure` checks `std::string::contains`, `std::string::resize_and_overwrite` and `std::stringstream::view`. Do not lower the language standard to work around -an old toolchain. Dependencies include Boost, ViennaRNA (>= 2.4.14), zlib, +an old toolchain. Dependencies include Boost, ViennaRNA (>= 2.7.0), zlib, pkg-config and Autotools; OpenMP is used by the default multithreaded build. [conda-build-env.yml](conda-build-env.yml) supplies library/build dependencies, but does not select the C++ compiler. Use the platform setup in CI when needed. @@ -154,8 +154,10 @@ Example::isComplete() const setup, constraints and preparation use `VrnaHandler::getSetupMutex()`. Prepared window accessibility computations can run concurrently with an OpenMP-enabled ViennaRNA build; global MFE/PF calls remain serialized. - Complete parameter loading before workers start, preserve the ViennaRNA - logger lock, and consult `doc/analysis/vienna-accessibility-thread-safety.md` + Complete parameter loading before workers start and preserve the mandatory + ViennaRNA logger lock. Retain full-call serialization for RNAlib builds + without OpenMP; this is a build-configuration boundary, not support for old + ViennaRNA releases. Consult `doc/analysis/vienna-accessibility-thread-safety.md` before changing these boundaries. - For recurrence or predictor changes, extend the relevant regression tests and short-sequence oracles (`PredictorTinyOracle_test.cpp` and diff --git a/ChangeLog b/ChangeLog index bc92b8f1..156d3ba2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -42,6 +42,9 @@ ## Technical changes and Optimizations +- require ViennaRNA >= 2.7.0 and its logger/prepared-window APIs at configure + time; install the logger lock for every supported version (PR #267) + - fix ViennaRNA parameter-cache races and enable parallel prepared window accessibility; retain serialization for global MFE/PF and non-OpenMP ViennaRNA builds, with a source audit and concurrency regressions (issue #265) @@ -135,6 +138,21 @@ energies, restricted partition sums, and trackers. ################################################################################ ################################################################################ +261015 Alexander Mitrofanov + * configure.ac, conda-build-env.yml : + * require ViennaRNA >= 2.7.0, the first release with vrna_log_lock_set() + * check header version and link the logger/prepared-window APIs during + configure, including builds using --disable-pkg-config + * IntaRNA/VrnaHandler, IntaRNA/AccessibilityVrna : + * install the logger lock unconditionally for supported ViennaRNA releases + and remove obsolete version guards; retain full-call serialization for + RNAlib builds without OpenMP + * README.md, AGENTS.md, doc/analysis/vienna-accessibility-thread-safety.md : + * align the dependency and synchronization contract; distinguish historical + 2.4.14 source evidence from the supported range and recorded runtime tests + + record minimum-version, release/debug and dependency-probe validation + * addresses review of https://github.com/BackofenLab/IntaRNA/pull/267 + 261014 Alexander Mitrofanov * tests/AccessibilityVrna_test.cpp : * replace std::jthread, unavailable in the macOS CI standard library, with diff --git a/README.md b/README.md index 211d4943..0d67fe7c 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,10 @@ dependencies: - libboost_system - libboost_iostreams - libboost_serialization -- [Vienna RNA package](http://www.tbi.univie.ac.at/RNA/) version >= 2.4.14 +- [Vienna RNA package](http://www.tbi.univie.ac.at/RNA/) version >= 2.7.0, + providing the logger locking API required for concurrent accessibility + calculations. Configure checks the version and required APIs, including + when `--disable-pkg-config` is used. - `pkg-config` for detailed version checks of dependencies - if [cloning from github](#instgithub): GNU autotools (automake, autoconf, ..) @@ -460,8 +463,10 @@ If `--threads=k != 1`, than *k* predictions are processed in parallel. A value o ViennaRNA accessibility calculations for independent sequences run concurrently when ViennaRNA was built with OpenMP support. IntaRNA serializes parameter and constraint setup to protect ViennaRNA's shared caches; builds without ViennaRNA -OpenMP support retain serial accessibility calculation. Global intramolecular -MFE/partition-function calls remain serialized. See the +OpenMP support retain serial accessibility calculation. IntaRNA installs a +separate ViennaRNA logger lock during handler initialization for every +supported version. Global intramolecular MFE/partition-function calls remain +serialized. See the [thread-safety audit](doc/analysis/vienna-accessibility-thread-safety.md) for the call sequence, source evidence, standalone reproducer and validation. diff --git a/conda-build-env.yml b/conda-build-env.yml index a038bb05..7f75876a 100644 --- a/conda-build-env.yml +++ b/conda-build-env.yml @@ -12,7 +12,7 @@ dependencies: - pkgconfig - zlib - boost-cpp - - viennarna>=2.4.14 + - viennarna>=2.7.0 - cxx-compiler - make - binutils diff --git a/configure.ac b/configure.ac index ae2df6d9..aeb4e0c9 100644 --- a/configure.ac +++ b/configure.ac @@ -5,8 +5,8 @@ AC_INIT([IntaRNA], [3.4.1], [], [intaRNA], [http://www.bioinf.uni-freiburg.de] ) # minimal required version of the boost library BOOST_REQUIRED_VERSION=1.50.0 -# minimal required version of the Vienna RNA library -VRNA_REQUIRED_VERSION=2.4.14 +# minimal Vienna RNA version providing the logger locking API +VRNA_REQUIRED_VERSION=2.7.0 # minimal required version of python interpreter (to install intarnapvalue) PYTHON_REQUIRED_VERSION=3.6 @@ -443,19 +443,41 @@ AS_IF([test x"$HAVE_PKG_CONFIG" = x"yes"], [ ]) AS_IF([test "$RNANOTFOUND" = "0"], [ - AC_MSG_CHECKING([for compilation with Vienna RNA package headers]) + AC_MSG_CHECKING([for ViennaRNA >= $VRNA_REQUIRED_VERSION with logger locking and prepared window folding]) OLD_CPPFLAGS=$CPPFLAGS - OLD_CXXFLAGS=$CXXFLAGS OLD_LDFLAGS=$LDFLAGS - AC_LANG_PUSH([C]) + intarna_saved_vrna_LIBS=$LIBS + AC_LANG_PUSH([C++]) CPPFLAGS="$CPPFLAGS $AM_CXXFLAGS" LDFLAGS="$LDFLAGS $AM_LDFLAGS" AS_IF([test "x$HAVE_PKG_CONFIG" = "xyes"], [ CPPFLAGS="$CPPFLAGS $VRNA_CFLAGS" - LDFLAGS="$LDFLAGS $VRNA_LIBS" + LIBS="$VRNA_LIBS $LIBS" + ], [ + LIBS="-lRNA $LIBS" ]) - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM([[#include ]],[[vrna_md_t tmp; vrna_md_copy(&tmp,&tmp);]])], + # Check headers even without pkg-config, and link the APIs used for concurrency. + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[ +extern "C" { +#include +#if VRNA_VERSION_MAJOR < 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR < 7) +#error ViennaRNA 2.7.0 or newer is required +#endif +#include +#include +#include +#include +} +]], [[ +vrna_md_t model; +vrna_md_set_default(&model); +vrna_log_lock_set(NULL, NULL); +vrna_fold_compound_t *fc = vrna_fold_compound("GCGC", &model, VRNA_OPTION_PF | VRNA_OPTION_WINDOW); +vrna_fold_compound_prepare(fc, VRNA_OPTION_PF | VRNA_OPTION_WINDOW); +vrna_probs_window(fc, 4, VRNA_PROBS_WINDOW_UP, NULL, NULL); +vrna_fold_compound_free(fc); +]])], [ AC_MSG_RESULT([yes]) RNANOTFOUND=0; @@ -467,9 +489,10 @@ AS_IF([test "$RNANOTFOUND" = "0"], [ RNANOTFOUND=1; ] ) - AC_LANG_POP([C]) + AC_LANG_POP([C++]) CPPFLAGS=$OLD_CPPFLAGS LDFLAGS=$OLD_LDFLAGS + LIBS=$intarna_saved_vrna_LIBS ]) # error output if ViennaRNA not found diff --git a/doc/analysis/vienna-accessibility-thread-safety.md b/doc/analysis/vienna-accessibility-thread-safety.md index 7f5668d0..a0cb70ef 100644 --- a/doc/analysis/vienna-accessibility-thread-safety.md +++ b/doc/analysis/vienna-accessibility-thread-safety.md @@ -8,15 +8,26 @@ uses compound-owned state and can run concurrently after synchronized setup. ## Reviewed upstream sources -The review covers the minimum supported ViennaRNA release, **2.4.14**, and the -**2.7.2** release used for the runtime experiments. Source links below are -pinned to immutable commits: +IntaRNA requires **ViennaRNA 2.7.0 or later**. The review covers that release's +required logging API and the **2.7.2** implementation used for the runtime +experiments. The earlier **2.4.14** review remains as historical evidence; +that release is no longer supported. Source links below are pinned to immutable +commits: | Release | Commit | Scope | | --- | --- | --- | -| 2.4.14 | [`0b8ca4c6604e7a8bea366af62d82f8f40cea49bf`](https://github.com/ViennaRNA/ViennaRNA/tree/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf) | Minimum supported API and implementation | +| 2.4.14 | [`0b8ca4c6604e7a8bea366af62d82f8f40cea49bf`](https://github.com/ViennaRNA/ViennaRNA/tree/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf) | Historical implementation comparison; unsupported | +| 2.7.0 | [`219394580aec203a9d6f0d5450021e22642d5a83`](https://github.com/ViennaRNA/ViennaRNA/tree/219394580aec203a9d6f0d5450021e22642d5a83) | Minimum supported release and public logger locking API | | 2.7.2 | [`1ffec79f5e258896160f7362ced8263450f371dc`](https://github.com/ViennaRNA/ViennaRNA/tree/1ffec79f5e258896160f7362ced8263450f371dc) | Installed runtime and current parameter cache implementation | +ViennaRNA 2.7.0 introduced the flexible logger, as recorded in its +[release notes](https://github.com/ViennaRNA/ViennaRNA/releases/tag/v2.7.0). +Its public [`vrna_log_lock_set()` interface](https://github.com/ViennaRNA/ViennaRNA/blob/219394580aec203a9d6f0d5450021e22642d5a83/src/ViennaRNA/utils/log.h#L379-L398) +lets IntaRNA protect diagnostics even when RNAlib has no built-in pthread +locking. This interface determines the minimum supported version. Configure +checks the header version and links the required logger and prepared-window +APIs. Using `--disable-pkg-config` follows the same requirement. + This is a review of the paths IntaRNA calls, not a blanket claim that RNAlib, its deprecated interfaces, or every future release is thread-safe. In particular, compiling RNAlib with OpenMP does not make all its static state @@ -70,7 +81,7 @@ and [matrix preparation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e2 | State or operation | Evidence | Consequence | | --- | --- | --- | | Parameter caches `p_pre`, `pf_pre`, and their initialization flags | [2.7.2 `params.c`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/params/params.c#L88-L217) enables `SPEEDUP_PARAMS`; both constructors rewrite cached model members and copy whole cached structures. Only `id` and `pf_id` are declared OpenMP thread-private. | Guard every IntaRNA entry to parameter construction with one shared mutex. Separate locks for accessibility and energy construction would not protect the same caches. | -| Parameter identifiers in older RNAlib | [2.4.14 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L88-L98), [identifier increment](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L552) | 2.4.14 lacks the newer caches, but its identifier is shared in builds without OpenMP. The same setup lock remains useful. | +| Parameter identifiers in historical RNAlib | [2.4.14 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L88-L98), [identifier increment](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L552) | The unsupported 2.4.14 release lacks the newer caches, but its identifier is shared in builds without OpenMP. This explains why removing the setup lock was not safe under the former version requirement. | | Global model defaults and legacy model variables | [2.7.2 `vrna_md_defaults_reset`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L346-L432), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/model.c#L311-L384) | Passing a model to `vrna_md_defaults_reset()` changes process-wide defaults; it does not merely reset that local object. Remove the worker-side call when every fold already receives its explicit model. Guard parameter-file loading/default reads during handler setup. | | `vrna_md_copy` | [2.7.2 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L196-L232) copies the supplied structure and its arrays. | Copying a stable handler model into worker-local storage needs no global lock. | | Sliding-window DP, auxiliary arrays, and constraint updates | [2.7.2 window implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/pf_window.c#L681-L958), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/LPfold.c#L669-L941) obtain arrays and models from the compound or allocate local helpers. | Independent prepared compounds can compute concurrently. Keep callback storage distinct and alive until each call returns. | @@ -92,32 +103,34 @@ IntaRNA itself is built without OpenMP. The parallel boundary is **create, constrain, and prepare while locked; compute the window recurrence with exclusive ownership of the compound**. Keep global MFE/PF calls synchronized. RNAlib builds without -OpenMP use a conservative full-call fallback. Do not concurrently mutate -the handler model, reload global parameter tables through external RNAlib -calls, modify the same compound, or replace callback state while a call is -running. IntaRNA's mutex cannot coordinate direct RNAlib calls made by other -libraries in an embedding application. +OpenMP use conservative full-call serialization. This is a property of the +RNAlib build configuration and is retained independently of the minimum +version. Do not concurrently mutate the handler model, reload global parameter +tables through external RNAlib calls, modify the same compound, or replace +callback state while a call is running. IntaRNA's mutex cannot coordinate +direct RNAlib calls made by other libraries in an embedding application. This guarantee concerns the ViennaRNA boundary. Native-thread library users must separately provide thread-safe IntaRNA/easylogging++ diagnostics, or leave verbose/performance logging disabled. The CLI's OpenMP logging synchronization is not a general native-thread logging contract. -For ViennaRNA 2.7 and later, handler initialization installs a process-lifetime -mutex callback once through `vrna_log_lock_set()`. This replaces the logger's -locking hook while preserving its output streams, levels, and message -callbacks. The logger mutex is distinct from the setup mutex because guarded -ViennaRNA operations can emit messages themselves. This protects diagnostics -even if RNAlib was built without pthread support. Embedding applications must -not reset or replace that hook while IntaRNA folds are running; callbacks must -not reenter folding. Earlier reviewed logging uses stdio directly rather than -the newer lazily initialized callback array. See the +For every supported ViennaRNA version, handler initialization installs a +process-lifetime mutex callback once through `vrna_log_lock_set()`. This +replaces the logger's locking hook while preserving its output streams, levels, +and message callbacks. The logger mutex is distinct from the setup mutex +because guarded ViennaRNA operations can emit messages themselves. This +protects diagnostics even if RNAlib was built without pthread support. +Embedding applications must not reset or replace that hook while IntaRNA folds +are running; callbacks must not reenter folding. There is no version-dependent +path that omits the logger lock. See the [public locking callback](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.h#L61-L70) and [hook setter](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.c#L284-L290). -The 2.4.14 source review establishes API availability and identifies its -state, but does not substitute for testing that release. Runtime comparisons -on 2.7.2 do not prove every intermediate release or build configuration safe. +The original runtime comparisons below used 2.7.2. The dependency-floor +revision also tested 2.7.0, as recorded below. Neither these finite tests nor +the historical 2.4.14 review establish runtime behavior for every supported +release or build configuration. Repeat the audit when changing the supported RNAlib range, adding model mutations after preparation, changing SHAPE integration, or adding callbacks. @@ -159,7 +172,8 @@ them as separate diagnostic processes, never as ordinary CI pass conditions: Observed results used ViennaRNA 2.7.2's static library with OpenMP enabled, GCC 14.4.0 (conda-forge build 14.4.0-7), and Linux x86-64 with CPU affinity 0-11. Runs requested eight workers; other build activity could share these -CPUs. No ViennaRNA 2.4.14 runtime was installed for the experiment. +CPUs. These recorded experiments did not test ViennaRNA 2.7.0 or 2.4.14 +runtimes. | Mode/profile | Completed repetitions | Exact probability comparisons | Result | | --- | ---: | ---: | --- | @@ -180,10 +194,11 @@ not a race-detector proof or a performance measurement. ## Integrated regressions and existing SHAPE limitations -GCC 14 release and `--enable-debug` builds each passed `make tests -j4`: -all seven test programs/scripts passed, including 75,901 API assertions in -69 test cases. The source base was `c5f1d51` on `master` (the repository has no -`main` branch). `git diff --check` and shell syntax validation also passed. +For the original synchronization change, GCC 14 release and `--enable-debug` +builds each passed `make tests -j4`: all seven test programs/scripts passed, +including 75,901 API assertions in 69 test cases. The source base was `c5f1d51` +on `master` (the repository has no `main` branch). `git diff --check` and shell +syntax validation also passed. `tests/AccessibilityVrna_test.cpp` compares all retained ED values (including the extra band used for dangling ends), ES matrices, initiation energies and @@ -227,6 +242,39 @@ both the parallel and serialized branches. It is **not** a runtime test of a non-OpenMP RNAlib: the linked library remained the installed OpenMP-enabled 2.7.2, and unchanged IntaRNA objects came from the release archive. +## Dependency-floor revision validation (2026-10-15) + +The revision requested in [PR #267](https://github.com/BackofenLab/IntaRNA/pull/267#issuecomment-6192018595) +was validated on Linux with GCC 14.4.0: + +| ViennaRNA | IntaRNA build | Command | Result | +| --- | --- | --- | --- | +| 2.7.0 | Release | `make tests -j4` | All seven suites passed | +| 2.7.2 | Release | `make tests -j2` | All seven suites passed | +| 2.7.2 | Debug | `make tests -j2` | All seven suites passed | + +Each run passed 75,901 API assertions in 69 cases, including the native-thread +and OpenMP accessibility comparisons. The 2.7.0 standalone diagnostic, +`vrna-window-threads setup-locked 4 3`, compared 1,011,504 probabilities over +24 mixed-model cases and three repetitions, with no parameter, probability, +or callback metadata mismatches. + +ViennaRNA 2.7.0 was built from its official release tarball as a static library +with OpenMP and pthread support. Its library-only, SVM-disabled build needed +`make 'libRNA_utils_la_CPPFLAGS=$(AM_CPPFLAGS)'` to restore an upstream omitted +include path; no upstream source was changed. Include and library search paths +placed this installation ahead of the existing 2.7.2 dependency prefix. The +IntaRNA executable's version output and compiler dependency files confirmed +that the minimum-version headers were selected. + +Six isolated configure probes also passed: a valid installation with +`--disable-pkg-config` and explicit static-library dependencies was accepted; +old pkg-config metadata, headers reporting 2.6.4 with and without pkg-config, +a missing logger declaration, and an unavailable logger link symbol were +rejected. These negative checks used controlled fixtures, not an installed +2.6.4 runtime. Installation, an external consumer exercising accessibility, +all 69 standalone public-header checks, and `git diff --check` passed. + ## End-to-end measurement The base and corrected binaries used the same GCC 14.4 release toolchain, diff --git a/src/IntaRNA/AccessibilityVrna.cpp b/src/IntaRNA/AccessibilityVrna.cpp index 8d53c829..d6f7ef1a 100644 --- a/src/IntaRNA/AccessibilityVrna.cpp +++ b/src/IntaRNA/AccessibilityVrna.cpp @@ -244,7 +244,8 @@ fillByRNAplfold( const VrnaHandler &vrnaHandler #ifdef VRNA_WITH_OPENMP // The expensive window recurrence and callback use per-compound storage. - // Retain full serialization for ViennaRNA builds without OpenMP support. + // OpenMP is a ViennaRNA build option, independent of its version; keep + // full serialization when the linked library was built without it. setupLock.unlock(); #endif diff --git a/src/IntaRNA/VrnaHandler.cpp b/src/IntaRNA/VrnaHandler.cpp index 594af3c6..79777843 100644 --- a/src/IntaRNA/VrnaHandler.cpp +++ b/src/IntaRNA/VrnaHandler.cpp @@ -16,14 +16,11 @@ extern "C" { #include #include #include -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) #include -#endif } namespace { -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) void lockVrnaLogging( const int lock, void * ) noexcept { @@ -36,7 +33,6 @@ lockVrnaLogging( const int lock, void * ) noexcept loggingMutex.unlock(); } } -#endif } @@ -55,11 +51,9 @@ VrnaHandler( Z_type temperature { std::lock_guard setupLock(getSetupMutex()); -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) // Install only the locking callback; preserve log outputs and user callbacks. static std::once_flag loggingInitialized; std::call_once(loggingInitialized, [] { vrna_log_lock_set(lockVrnaLogging, nullptr); }); -#endif // load parameters if ( !vrnaParamFile.empty() ) { diff --git a/src/IntaRNA/VrnaHandler.h b/src/IntaRNA/VrnaHandler.h index b5ad46f5..4e3ea4f7 100644 --- a/src/IntaRNA/VrnaHandler.h +++ b/src/IntaRNA/VrnaHandler.h @@ -39,8 +39,8 @@ class VrnaHandler { /** * Construction with global VRNA folding parameter setup * Complete parameter loading before starting concurrent folding computations. - * For ViennaRNA 2.7 and newer, the first handler also installs a mutex for - * logging without changing output callbacks. Retain this logging lock for + * The first handler also installs a mutex for logging without changing + * output callbacks. Retain this logging lock for * all subsequent concurrent folding; logger resets remove it. * * @param vrnaParamFile name of a VRNA parameter file to be used for