# Scoped thread using modern C++

I wrote this simple scoped thread to use it as a class member to ensure the the thread running on a class method doesn't continue running after the class has been destructed. Also to spawn threads from the main() function to ensure that I don't forget any running threads upon exit .

This can be achieved by calling join() method of the C++11 std::thread but I wanted a class that will automatically do this.

I didn't implement detach() function because I think this kills the purpose of a scoped thread; is this right?

I'd like to know if there is any bug, memory leak, other errors ...

#include <tuple>
#include <process.h>

extern "C"
{

__declspec(dllimport)
BOOL
__stdcall
CloseHandle(
HANDLE hObject
);

__declspec(dllimport)
DWORD
__stdcall
WaitForSingleObject(
HANDLE hHandle,
DWORD dwMilliseconds
);

__declspec(dllimport)
DWORD
__stdcall
void
);
}
template <class T, auto CloseFn>
struct HandleDeleter
{
using pointer = T;
void operator ()(pointer handle) { CloseFn(handle); }
};

using Handle = std::unique_ptr<void, HandleDeleter<HANDLE, CloseHandle>>;

inline std::error_code make_system_error(DWORD val) noexcept
{
return std::error_code{ static_cast<int>(val), std::system_category() };
}

inline std::error_code make_system_error(int val) noexcept
{
return std::error_code{ val, std::system_category() };
}

namespace LIB_NAMESPACE
{

namespace detail
{
template <class Func, class ... Args>
{
public:

ThreadParams(Func&& func, Args && ... args)
: fn{ std::forward<Func>(func) }, args_tuple{ std::forward<Args>(args)... }
{}

void Invoke()
{
std::apply(fn, args_tuple);
}

private:
Func fn;
std::tuple<Args...> args_tuple;
};

template <class ParamsType>
{
auto params = reinterpret_cast<ParamsType*>(args);
params->Invoke();
delete params;
return 0;
}

}

{
public:

using native_handle_type = Handle;
{
public:

thread_id(unsigned int id) : id_{ id } {}

{
other.invalidate();
}

{
id_ = other.id_;
other.invalidate();
return *this;
}

{
id_ = 0;
}

unsigned int id() const { return id_; }

{
return id() == other.id();
}

{
return id() != other.id();
}

{
return id() > other.id();
}

{
return id() >= other.id();
}

{
return id() < other.id();
}

{
return id() <= other.id();
}

private:

void invalidate() { id_ = 0; }

unsigned int * id_ptr() { return &id_; }

unsigned int id_ = 0;
};

template <class Fn, class ... Args>
scoped_thread(Fn&& fn, Args && ... args)
{
ParamsType* params = new ParamsType{ std::forward<Fn>(fn), std::forward<Args>(args)... };
thd_handle.reset(
);
if (!thd_handle)
{
delete params;
throw std::system_error(errno, std::system_category());
}
}

{
if (joinable())
std::terminate();
return *this;
}

native_handle_type& native_handle() noexcept { return thd_handle; }

const native_handle_type& native_handle() const noexcept { return thd_handle; }

thread_id get_id() const noexcept { return id; }

bool joinable() const noexcept
{
}

void join()
{
check_if_valid_join();
WaitForSingleObject(thd_handle.get(), std::numeric_limits<DWORD>::max());
thd_handle.reset();
id.invalidate();
}

{
if (joinable())
join();
}

private:

void check_if_valid_join()
{
if (!thd_handle)
throw std::system_error(std::make_error_code(std::errc::no_such_process));
if (!joinable())
throw std::system_error(std::make_error_code(std::errc::invalid_argument));
}

native_handle_type thd_handle;
};

}

• I might be missing something, but why couldn't scoped_thread be implemented in terms of std::thread? Feb 11 '20 at 17:51
• It could, but I wanted to make use of the new language features and to be more familiar with templates Feb 12 '20 at 11:36

Overall I think the code is pretty good and your use of the more modern features of C++ looks OK.

Still there are some things that could be improved. In no particular order I offer the following suggestions:

• The idea of a scoped_thread is good (also discussed in Effective Modern C++ item 37). Even if you want to implement your own std::thread replacement (for learning purposes or otherwise), I'd recommend splitting that part out and implementing scoped_thread in terms of it. That way you can re-use both classes in other contexts.
• I'd probably use either the functions from process.h or the raw Win32 API (i.e. CreateThread) not mix them.
• Speaking of Win32 API functions: Just include windows.h, don't do the declarations on your own.
• I realize including windows.h in a header file like this is pollutes the global namespace, so I'd create wrapper functions like void native_start_thread(void (*)(void*)) etc. and place them in an accompanying C++ file.
• In modern C++ naked calls to delete are a code smell. You should always have the pointers owned by a std::unique_ptr. Of course you have to be careful in this case to not double delete objects. What I mean is: In ThreadFunc hand ownership to a std::unique_ptr and in the scoped_thread constructor store params in a unique_ptr. Use get() when creating the thread and then release() when the thread has been constructed (and thus has taken ownership of the memory).
• You should check the return value of WaitForSingleObject
• The code could use some more comments. For instance I had to look up whether 0 could be a valid thread id (it can't, but I couldn't remember that was the case).
• I can't see anything wrong with the code in this regard, but I'm not too fond of thread ownership actually being tied to the thread id.
• Again, I don't see any immediate problems, but I'd probably make sure to null out the thread handle in the moved from thread (to at least aid in debugging). This would mean using a non-defaulted move-constructor, and then I'd probably implement it in terms of the move assignment operator (which then wouldn't/couldn't use placement new, which again I think works fine, but seems "overkill" here).
• "I'd probably use either the functions from process.h or the raw Win32 API (i.e. CreateThread) not mix them" . the docs recommends to start any thread that uses c runtime library using functions from process.h in order to avoid a small memory leak when the thread exits . for the last remark : it's already done because the thread handle is a smart handle (handle in a smart pointer) so the default unique_ptr move assignment operator nulls out (invalidates) the handle once moved from to ensure one object manages the handle . thanks for your tips . Feb 12 '20 at 18:04
• @dev65: Sorry my bad on both counts. I mistakenly though the need to use _beginthread was gone, but it seems like that's not the case when linking statically to the CRT. And in the heat of battle I'd forgotten that native_handle_type was a smart pointer - I'll amend my post later to reflect this. Feb 12 '20 at 18:25