On Thu, 12 Nov 2026 at 17:41, Malcolm Parsons <[email protected]> wrote:
>
> The constructor for an immediately-owned unique_lock initializes the
> ownership flag to false and then calls the checked lock() modifier, which
> sets the flag to true. The constructor then sets it to true again. The
> destructor similarly calls the checked unlock() modifier after already
> testing ownership.
>
> Match the implementation pattern used by shared_lock: construct the object
> directly in its final ownership state, call the mutex operations directly
> during construction and destruction, and remove unlock()'s redundant mutex
> pointer check after ownership has been established. This avoids duplicated
> checks and exception paths without changing observable behavior.
>
> No new test is needed because the change has no observable behavior. The
> existing tests cover construction, destruction, explicit unlocking, and
> condition_variable_any waits.
>
> Tested on x86_64-pc-linux-gnu with no regressions.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/unique_lock.h (unique_lock::unique_lock): Construct
> directly in the owned state and call the mutex directly.
> (unique_lock::~unique_lock): Unlock the mutex directly.
> (unique_lock::unlock): Remove redundant mutex pointer check.
>
> Assisted-by: OpenAI Codex
> ---
> libstdc++-v3/include/bits/unique_lock.h | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/unique_lock.h
> b/libstdc++-v3/include/bits/unique_lock.h
> index 51596ef31..162be4b62 100644
> --- a/libstdc++-v3/include/bits/unique_lock.h
> +++ b/libstdc++-v3/include/bits/unique_lock.h
> @@ -70,11 +70,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> [[__nodiscard__]]
> explicit unique_lock(mutex_type& __m)
> - : _M_device(std::__addressof(__m)), _M_owns(false)
> - {
> - lock();
> - _M_owns = true;
> - }
> + : _M_device(std::__addressof(__m)), _M_owns(true)
> + { __m.lock(); }
>
> unique_lock(mutex_type& __m, defer_lock_t) noexcept
> : _M_device(std::__addressof(__m)), _M_owns(false)
> @@ -111,7 +108,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ~unique_lock()
> {
> if (_M_owns)
> - unlock();
> + _M_device->unlock();
This removes the check for non-null _M_device, but it should be
impossible for _M_owns to be true and _M_device to be null.
> }
>
> unique_lock(const unique_lock&) = delete;
> @@ -198,11 +195,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> if (!_M_owns)
> __throw_system_error(int(errc::operation_not_permitted));
> - else if (_M_device)
> - {
> - _M_device->unlock();
> - _M_owns = false;
> - }
And this does the same. Again, this should be fine in a correct
program, but it could change the behaviour for a program which has
somehow broken the unique_lock invariants. The technically redundant
check for _M_device avoids a segfault, or optimizations that assume
_M_device must be non-null because it's dereferenced.
How much would it hurt the codegen to do `if (!_M_owns || !_M_device)`
in the condition for the throwing path?
And similarly, check `if (_M_owns && _M_device)` in the destructor.
Or just add __glibcxx_assert(_M_device) immediately before the
_M_device->unlock() calls.
> + _M_device->unlock();
> + _M_owns = false;
> }
>
> void
> --
> 2.43.0
>
