Resolve LAC_THREADS in the CLI, not inside the library#34
Merged
Conversation
The encoder and decoder previously called resolve_thread_limit() deep inside
encode()/decode(), which read getenv("LAC_THREADS"). That made a library call
depend on ambient process state and was not safe against concurrent setenv.
Move environment resolution to the CLI: main.cpp now resolves --threads (which
takes precedence) and otherwise LAC_THREADS via resolve_cli_thread_count(), and
passes an explicit thread count to the library. The encoder/decoder use that
count directly (0 = auto) and no longer include thread_limit.hpp.
CLI behavior is unchanged. Direct library use no longer reads the environment;
README is updated to state that LAC_THREADS applies to lac_cli (the unit tests
set their own thread counts via set_thread_count).
Refs #27
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 of
docs/refactor-roadmap.md(#27): stop the library from reading the environment.The encoder and decoder previously called
resolve_thread_limit()insideencode()/decode(), which readgetenv("LAC_THREADS"). A library call should not depend on ambient process state, and that read is not safe against a concurrentsetenv.Changes
resolve_thread_limit()fromthread_limit.hpp(keep the pureparse_thread_limit()string parser).Block/LACencoder and decoder now use their explicitthread_countdirectly (0 = auto) and no longer includethread_limit.hpp.main.cppresolves the thread count once viaresolve_cli_thread_count():--threadswins, otherwiseLAC_THREADS, and passes an explicit value to the library (encode + both decode paths).LAC_THREADSapplies to thelac_clibinary (and CLI subprocess tests); internal codec unit tests set their own thread counts.Behavior
--threads=Nstill overridesLAC_THREADS, both still cap workers.set_thread_count()(0 = auto). This is the intended point of the change.docs/supported-formats.mdalready documents--threads/LAC_THREADSas CLI options, which remains accurate.Deferred
LAC_DEBUG_*env macros, and unsynchronized debug logging) is not in this PR. TheLAC_DEBUG_*reads are debug-build-only and tie into Decompose the monolithic block/frame encode functions #25/Make multithreaded encoder startup exception-safe #5; left for a follow-up. Keeping Unify debug/configuration mechanism and remove hidden env coupling from the library #27 open for that.Verification (local, macOS)
resolve_thread_limitis gone andsrc/codec/contains nogetenv.ctest: 2/2 suites pass, including the explicit thread-count assertions inrun_thread_limit_tests/run_decoder_thread_tests(which useset_thread_count, not the environment).lac_cli selftest: passes.Refs #27