[Bug libstdc++/57641] New: std::timed_mutex.try_lock_until() is broken

[Bug libstdc++/57641] New: std::timed_mutex.try_lock_until() is broken

[mustrumr97 at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

            Bug ID: 57641
           Summary: std::timed_mutex.try_lock_until() is broken
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mustrumr97 at gmail dot com

It uses the duration since the time_point's clock's epoch. What if it's not the
right clock? Different clocks may have different epochs. On my computer, the
function works OK if the time_point is from high_resolution_clock but doesn't
work if it's from steady_clock.

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I agree that code assumes the epochs are the same, but what you describe
doesn't make sense since high_resolution_clock and steady_clock have the same
epoch in our implementation.  Are you seeing the first problem described at PR
54562?

Do you have a testcase?

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[mustrumr97 at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

--- Comment #2 from Hristo Venev <mustrumr97 at gmail dot com> ---
Am I very stupid, or is

#include <mutex>
#include <chrono>
using Clock=std::chrono::steady_clock;
int main(){
    std::timed_mutex m;
    m.lock();
    Clock::time_point tp=Clock::now()+std::chrono::seconds(2);
    if(m.try_lock_until(tp)) return 1;
    return 0;
}

supposed to run for ~2s and return 0?
With clang++ -stdlib=libc++ it works as I expect.
With clang++ -stdlib=libstdc++ and with g++ it returns 1 after 0.001s.

The result is the same for std::chrono::high_resolution_clock.

The test from cppreference.com is very similar.
http://en.cppreference.com/w/cpp/thread/timed_mutex/try_lock_until

What the hell is going on?

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Testcase using clock with an earlier epoch:

#include <chrono>
#include <thread>
#include <mutex>
#include <assert.h>

namespace C = std::chrono;

struct clock
{

    typedef C::system_clock::rep rep;
    typedef C::system_clock::period period;
    typedef C::system_clock::duration duration;
    typedef std::chrono::time_point<clock> time_point;
    static constexpr bool is_steady = C::system_clock::is_steady;

    static time_point now() {
        return time_point(C::system_clock::now().time_since_epoch() +
C::seconds(10));
    }
};

std::timed_mutex mx;

void f()
{
    mx.try_lock_until(clock::now() + C::milliseconds(1));
}

int main()
{
    std::lock_guard<std::timed_mutex> l(mx);
    auto start = C::system_clock::now();
    std::thread t(f);
    t.join();
    auto stop = C::system_clock::now();
    assert( (stop - start) < C::seconds(9) );
}

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's undefined behaviour because you try to lock the same mutex twice in the
same thread, see my testcase for a well-defined way to do it, calling
try_lock_until in a new thread.

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-06-18
     Ever confirmed|0                           |1

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The timed mutex requirements [thread.timedmutex.requirements] say:

The expression m.try_lock_until(abs_time) shall be well-formed and have the
following semantics:
Requires: If m is of type std::timed_mutex, the calling thread does not own the
mutex.

Anyway, confirming as there's a bug here relating to clocks with different
epochs, but it's easy enough to fix, I can look into it.  The same problem
exists with the private mutex type defined in std::shared_mutex

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.2

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk, I'll fix this for 4.8.2 as well.

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Author: redi
Date: Mon Oct  7 23:21:58 2013
New Revision: 203256

URL: http://gcc.gnu.org/viewcvs?rev=203256&root=gcc&view=rev
Log:
    PR libstdc++/57641
    * include/std/mutex (timed_mutex, recursive_timed_mutex): Add
    overloaded _M_try_lock_until to handle conversion between different
    clocks. Replace constrained __try_lock_for_impl overloads with
    conditional increment.
    * testsuite/30_threads/timed_mutex/try_lock_until/57641.cc: New.

Added:
   
branches/gcc-4_8-branch/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc
Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/std/mutex

[Bug libstdc++/57641] std::timed_mutex.try_lock_until() is broken

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57641

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
fixed for 4.8.2