boostorg / thread

Boost.org thread module
http://boost.org/libs/thread
197 stars 162 forks source link

Fix issue #366 (adding testcase) #369

Closed sehe closed 2 years ago

sehe commented 2 years ago

With reference to https://github.com/boostorg/thread/issues/366 and https://github.com/boostorg/thread/pull/367

This fix is narrower than the linked PR in that it only prevents interruption exceptions from being raised inside the thread_guard destructor.

sehe commented 2 years ago

Need to also have a look at scope_guard and scoped_thread (thanks for the hint @elsamuko)

pdimov commented 2 years ago

As far as I can see the minimal test just needs to be

#include "boost/thread/thread_guard.hpp"
#include "boost/thread.hpp"
#include <iostream>

static void inner_thread_func()
{
    boost::this_thread::sleep_for(
        boost::chrono::hours(1) );
}

static void outer_thread_func()
{
    boost::thread inner( inner_thread_func );
    boost::thread_guard<boost::interrupt_and_join_if_joinable> guard( inner );
}

static void double_interrupt()
{
    boost::thread outer( outer_thread_func );
    outer.interrupt();
    outer.join();
}

int main()
{
    std::cout << "Start" << std::endl;
    double_interrupt();
    std::cout << "End" << std::endl;
}

and, correspondingly,

#include "boost/thread/scoped_thread.hpp"
#include "boost/thread.hpp"
#include <iostream>

static void inner_thread_func()
{
    boost::this_thread::sleep_for(
        boost::chrono::hours(1) );
}

static void outer_thread_func()
{
    boost::scoped_thread<boost::interrupt_and_join_if_joinable> inner( inner_thread_func );
}

static void double_interrupt()
{
    boost::scoped_thread<boost::interrupt_and_join_if_joinable> outer( outer_thread_func );
}

int main()
{
    std::cout << "Start" << std::endl;
    double_interrupt();
    std::cout << "End" << std::endl;
}

I'm not sure we need to do anything about scoped_guard, although I suppose it won't hurt to disable interrupts in its destructor, too. There's no scenario in which you want them not disabled.

pdimov commented 2 years ago

Now that I look at it, maybe we can replace

static void double_interrupt()
{
    boost::thread outer( outer_thread_func );
    outer.interrupt();
    outer.join();
}

with

static void double_interrupt()
{
    boost::thread outer( outer_thread_func );
    boost::thread_guard<boost::interrupt_and_join_if_joinable> guard( outer );
}

for symmetry.

elsamuko commented 2 years ago

@sehe @pdimov scope_guard was just a helper class in my example code and is not part of boost::thread. The thread functors are currently used in

pdimov commented 2 years ago

Well, we can't do anything about user-defined scope guards.

pdimov commented 2 years ago

All right, we need a third test for strict_scoped_thread, then.

sehe commented 2 years ago

I'll add the test and fix for *scoped_thread

pdimov commented 2 years ago

Should be fixed with https://github.com/boostorg/thread/commit/f71e0f1645413c3291e37d766e5b04bc57c6e132 and https://github.com/boostorg/thread/commit/8db325363b8e91de23c062ec8964bb605ad89f11.