Thanks! I've committed this.
One last note (not important enough to worry about, but I just wanted to
point out for future reference) that where you did
osv::clock::wall::duration time((std::chrono::seconds(0)));
tmr.set(time);
A simple
tmr.set(std::chrono::seconds(0));
Would have been enough.
That's the beauty of the std::chrono API, that it makes is easy to
implicitly convert different types and resolutions of durations to what is
need.
If you're wondering how this works, well, in our implementation, we have
the template function:
template <class Rep, class Period>
void set(std::chrono::duration<Rep, Period> duration) {
set(osv::clock::uptime::now() + duration);
}
So you can see that *any* type of duration is accepted, it doesn't
specifically need to be an osv::clock::wall:duration (which has a specific
resolution), so you didn't have to explicitly create a variable with that
typ.e
--
Nadav Har'El
[email protected]
On Thu, Dec 8, 2016 at 8:45 AM, 'rean' via OSv Development <
[email protected]> wrote:
> Signed-off-by: rean <[email protected]>
> ---
> core/semaphore.cc | 8 +++++++-
> modules/tests/Makefile | 2 +-
> tests/tst-sem-timed-wait.cc | 45 ++++++++++++++++++++++++++++++
> +++++++++++++++
> 3 files changed, 53 insertions(+), 2 deletions(-)
> create mode 100644 tests/tst-sem-timed-wait.cc
>
> diff --git a/core/semaphore.cc b/core/semaphore.cc
> index 73732b8..31fda35 100644
> --- a/core/semaphore.cc
> +++ b/core/semaphore.cc
> @@ -48,7 +48,13 @@ bool semaphore::wait(unsigned units, sched::timer* tmr)
>
> // if wr.owner, it's a timeout - post() didn't wake us and didn't
> decrease
> // the semaphore's value for us. Note we are holding the mutex, so
> there
> - // can be no race with post().
> + // can be no race with post(). To clean up we should remove the
> + // wait record (local variable) that we just pushed onto _waiters
> + // (via push_back)
> + if (wr.owner) {
> + _waiters.erase(_waiters.iterator_to(wr));
> + }
> +
> return !wr.owner;
> }
>
> diff --git a/modules/tests/Makefile b/modules/tests/Makefile
> index fb8d001..6e3aea7 100644
> --- a/modules/tests/Makefile
> +++ b/modules/tests/Makefile
> @@ -84,7 +84,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so
> tst-bsd-evh.so \
> tst-namespace.so tst-without-namespace.so payload-env.so \
> payload-merge-env.so misc-execve.so misc-execve-payload.so
> misc-mutex2.so \
> tst-pthread-setcancelstate.so tst-syscall.so tst-pin.so tst-run.so
> \
> - tst-ifaddrs.so tst-pthread-affinity-inherit.so
> + tst-ifaddrs.so tst-pthread-affinity-inherit.so
> tst-sem-timed-wait.so
>
> # libstatic-thread-variable.so tst-static-thread-variable.so \
>
> diff --git a/tests/tst-sem-timed-wait.cc b/tests/tst-sem-timed-wait.cc
> new file mode 100644
> index 0000000..f59d294
> --- /dev/null
> +++ b/tests/tst-sem-timed-wait.cc
> @@ -0,0 +1,45 @@
> +#include <osv/semaphore.hh>
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +unsigned int tests_total = 0, tests_failed = 0;
> +
> +void report(const char* name, bool passed)
> +{
> + static const char* status[] = {"FAIL", "PASS"};
> + printf("%s: %s\n", status[passed], name);
> + tests_total += 1;
> + tests_failed += !passed;
> +}
> +
> +int main(void)
> +{
> + printf("Starting sem_timed_wait test\n");
> +
> + // Basic flow for test
> + // 1) Create a semaphore (initialized to 0)
> + // 2) Do a timed-wait on it
> + // 3) We're never signaled/woken so our stack-allocated wait_record
> remains
> + // on the semaphore's waiters list
> + //
> + // In the failure case the end result is a stacktrace that looks like:
> + // Assertion failed: !hook.is_linked()
> + //(/usr/include/boost/intrusive/detail/generic_hook.hpp:
> destructor_impl: 47)
> +
> + //[backtrace]
> + //0x0000000000225a48 <__assert_fail+24>
> + //0x00000000003c40e9 <???+3948777>
> + //0x00000000003c4242 <semaphore::wait(unsigned int, sched::timer*)+98>
> + //0x0000100000c01057 <???+12587095>
> +
> + semaphore sem(0);
> + sched::timer tmr(*sched::thread::current());
> + osv::clock::wall::duration time((std::chrono::seconds(0)));
> + tmr.set(time);
> + bool ret_val = sem.wait(1, &tmr);
> + report("sem_timedwait\0", ret_val == false);
> + printf("SUMMARY: %u tests / %u failures\n", tests_total, tests_failed);
> + return tests_failed == 0 ? 0 : 1;
> +}
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
