boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
200 stars 161 forks source link

Fixes and Cleanup #167

Closed austin-beer closed 6 years ago

austin-beer commented 6 years ago

I'm hoping this PR will fix the AppVeyor failures that the last PR introduced. I have been building with Visual Studio 2015 but I switched over to Visual Studio 2010 to better catch issues like this in the future.

viboes commented 6 years ago

Ok, I will add an old version of msvc into AppVeyor to check this kind of backward compatibility issues. Maybe you can do it on your branch.

viboes commented 6 years ago

There are a lot of errors in Windows and also on MacOs :(

austin-beer commented 6 years ago

Yep, I'm working on those as we speak.

viboes commented 6 years ago

The issue from

libs/thread/test/sync/mutual_exclusion/sync_pq/pq_single_thread_pass.cpp(58): test 'diff < milliseconds(600) && diff > milliseconds(500)' failed in function 'void test_pull_for()'

is not your fault.

The implementation/interface of sync_priority_queue is wrong. I'll try to fix it.

Hrr, there are too much wrong time related interfaces in Boost.Thread.

viboes commented 6 years ago

I don't remember why I dded such time-related functions as the C++ proposal it is based on (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0260r1.html) don't provides such functions.

viboes commented 6 years ago

If you believe your PR is ready to merge I'll do.

viboes commented 6 years ago

Could you give a last try with

  BOOST_TEST(diff < milliseconds(650) && diff > milliseconds(500));

I suspect that now that we could be pooling (at least in MacOs) waiting for 500ms could be very easily wait for more than 600ms.

viboes commented 6 years ago

As you see I've already merged your PR.

austin-beer commented 6 years ago

Thanks. I've been out of the office all weekend so I wasn't able to take care of this. Thanks for taking a look at the pq_single_thread_pass issues.