Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Tricky bug

93 views
Skip to first unread message

Bonita Montero

unread,
Dec 13, 2020, 4:39:47 AM12/13/20
to
I had a nice bug: I've got a base-class with a pure virtual function
which is overridden in a derived class. This function is virtually
called by a thread from a thread pool (through a static proxy-func-
tion). In the destructor of the base-class I wait for all threads
from the thread pool to have finished their work and that there are
no more items in their work-queue.
What I missed was, that as soon as the base-class destructor is called
the virtual fuction pointer is adjusted to the pointer of the base-class
(I'm wondering that is there any because a base-class with a pure vir-
tual function shouldn't have a virtual method table at all). So before
I could wait for the threads to finish all work the null pure virtual
function got called. ;-)
Interestingly this doesn't happen if I don't compile the code as debug
-code. So the compiler indirectly gives me a hint here for a possible
bug.

Richard Damon

unread,
Dec 13, 2020, 8:05:53 AM12/13/20
to
The vtable change is because once you enter the base class destructor,
there isn't any derived class to enter anymore (it has been destructed),
so the object is now just of the base class.

This says either you have an ownership issue with the object, where you
are destroying it before all the users are done with it, or the wait in
the destructor is surperfilous, as all the threads should be done before
you got there.

Maybe you need that wait in each of the destructors of the derived
classes (maybe calling a method in the base), but even that is
technically late as you shouldn't be doing even that if there are still
things referring to it.

Marcel Mueller

unread,
Dec 13, 2020, 8:35:58 AM12/13/20
to
Am 13.12.20 um 14:05 schrieb Richard Damon:
>> Interestingly this doesn't happen if I don't compile the code as debug
>> -code. So the compiler indirectly gives me a hint here for a possible
>> bug.
>
> The vtable change is because once you enter the base class destructor,
> there isn't any derived class to enter anymore (it has been destructed),
> so the object is now just of the base class.

Yes that is the way C++ initializes and destroys the objects.

While formally correct I never found a practical use case for this behavior.
In contrast it creates the need for initialization or deinitialization
functions that need to be called manually for every class instance. So
it is correct but useless. From my point of view a language must be
first of all useful.


Marcel

Bonita Montero

unread,
Dec 13, 2020, 8:40:11 AM12/13/20
to
> The vtable change is because once you enter the base class destructor,
> there isn't any derived class to enter anymore (it has been destructed),
> so the object is now just of the base class.

As you can read from my posting I already know that.

> Maybe you need that wait in each of the destructors of the derived
> classes (maybe calling a method in the base), but even that is
> technically late as you shouldn't be doing even that if there are
> still things referring to it.

I'm using a simple function-pointer now.

Paavo Helde

unread,
Dec 13, 2020, 9:00:59 AM12/13/20
to
13.12.2020 11:39 Bonita Montero kirjutas:
> I had a nice bug: I've got a base-class with a pure virtual function
> which is overridden in a derived class. This function is virtually
> called by a thread from a thread pool (through a static proxy-func-
> tion). In the destructor of the base-class I wait for all threads
> from the thread pool to have finished their work and that there are
> no more items in their work-queue.
> What I missed was, that as soon as the base-class destructor is called
> the virtual fuction pointer is adjusted to the pointer of the base-class
> (I'm wondering that is there any because a base-class with a pure vir-
> tual function shouldn't have a virtual method table at all). So before
> I could wait for the threads to finish all work the null pure virtual
> function got called. ;-)

That's what std::shared_ptr is for. By its design you cannot have such
problems (either during construction or destruction) if you access your
shared object via std::shared_ptr pointers only. Whenever you have a
shared_ptr to an object, you can be sure that it points to the most
derived class object (because a shared_ptr cannot be constructed before
the most derived class object is ready, and the destructor of the most
derived class is not evoked while there is a shared_ptr alive).

Just stop trying to be too clever for no good, always access your shared
objects via shared_ptr, and adjust the rest of the code to cope with
that simple rule. That's it, a major source of nasty heisenbugs is gone!

Juha Nieminen

unread,
Dec 13, 2020, 9:08:22 AM12/13/20
to
Paavo Helde <myfir...@osa.pri.ee> wrote:
> That's what std::shared_ptr is for. By its design you cannot have such
> problems (either during construction or destruction) if you access your
> shared object via std::shared_ptr pointers only. Whenever you have a
> shared_ptr to an object, you can be sure that it points to the most
> derived class object (because a shared_ptr cannot be constructed before
> the most derived class object is ready, and the destructor of the most
> derived class is not evoked while there is a shared_ptr alive).

I don't understand what std::shared_ptr has anything to do with this.

The problem was that the destructor of the base class was calling a pure
virtual method (something that a good compiler should warn about, but
it's not an enforceable error).

I don't think std::shared_ptr would have remedied that in any way.
When the last std::shared_ptr object is destroyed, it will destroy the
managed object, and the problem will happen. std::shared_ptr doesn't
somehow magically guard against that problem.

Bonita Montero

unread,
Dec 13, 2020, 9:13:30 AM12/13/20
to
> The problem was that the destructor of the base class was calling a pure
> virtual method (something that a good compiler should warn about, but
> it's not an enforceable error).

Not the destructor but another function from another thread to the same
object.

Bonita Montero

unread,
Dec 13, 2020, 9:14:18 AM12/13/20
to
> That's what std::shared_ptr is for. By its design you cannot have such
> problems (either during construction or destruction) if you access your
> shared object via std::shared_ptr pointers only. Whenever you have a
> shared_ptr to an object, you can be sure that it points to the most
> derived class object (because a shared_ptr cannot be constructed before
> the most derived class object is ready, and the destructor of the most
> derived class is not evoked while there is a shared_ptr alive).

shared_ptr<> has nothing to do with my issue.

Paavo Helde

unread,
Dec 13, 2020, 9:28:19 AM12/13/20
to
13.12.2020 16:07 Juha Nieminen kirjutas:
> Paavo Helde <myfir...@osa.pri.ee> wrote:
>> That's what std::shared_ptr is for. By its design you cannot have such
>> problems (either during construction or destruction) if you access your
>> shared object via std::shared_ptr pointers only. Whenever you have a
>> shared_ptr to an object, you can be sure that it points to the most
>> derived class object (because a shared_ptr cannot be constructed before
>> the most derived class object is ready, and the destructor of the most
>> derived class is not evoked while there is a shared_ptr alive).
>
> I don't understand what std::shared_ptr has anything to do with this.
>
> The problem was that the destructor of the base class was calling a pure
> virtual method (something that a good compiler should warn about, but
> it's not an enforceable error).

If you call a pure virtual method from dtor, you get guaranteed crash
each time, not a nasty timings-dependent multithreading bug which Bonita
was describing.

>
> I don't think std::shared_ptr would have remedied that in any way.
> When the last std::shared_ptr object is destroyed, it will destroy the
> managed object, and the problem will happen. std::shared_ptr doesn't
> somehow magically guard against that problem.

Yes it does. When you call a member function of an object via
std::shared_ptr then it means its destructor has not yet started
(neither in this nor in another thread), so there is no danger to call a
pure virtual method.

Bonita Montero

unread,
Dec 13, 2020, 12:55:58 PM12/13/20
to
Here's the current state of my thread-pool-class:

// debug_exceptions.h

#pragma once
#if !defined try_debug
#if !defined(NDEBUG)
#define try_debug try
#else
#define try_debug
#endif
#endif
#if !defined catch_debug
#if !defined(NDEBUG)
#define catch_debug catch( ... )
#else
#define catch_debug if( false )
#endif
#endif

// thread_pool.h

#pragma once
#include <thread>
#include <deque>
#include <future>
#include <utility>
#include <functional>
#include <memory>
#include <mutex>
#include <condition_variable>
#include <cassert>
#include <chrono>
#include <cstdint>
#include "debug_exceptions.h"

#if defined(_MSC_VER)
#pragma warning(disable: 26117)
#endif

struct thread_pool
{
using void_task = std::packaged_task<void()>;
using task_queue = std::deque<void_task>;
thread_pool( unsigned maxThreads );
template<typename Rep, typename Period>
thread_pool( unsigned maxThreads,
std::chrono::duration<Rep, Period> const &timeout );
~thread_pool();
template<typename F, typename ... Args>
std::uint64_t enqueue_task( F &&f, Args &&...args );
bool cancel( std::uint64_t queueId );
void wait_idle();
void resize( unsigned maxThreads );
task_queue clear_queue();
template<typename Rep, typename Period>
void set_timeout( std::chrono::duration<Rep, Period> const
&timeout );
bool fail();
void clear_fail();
template<typename F, typename ... Args>
void idle_callback( F &&f, Args &&...args );
void reset_callbacks();
std::size_t to_process();

private:
std::mutex m_mtx;
unsigned m_maxThreads;
std::chrono::milliseconds m_timeout;
unsigned m_stop;
unsigned m_nThreads;
unsigned m_nThreadsRunning;
std::condition_variable m_queueCv;
task_queue m_taskQueue;
std::uint64_t m_firstQueueId;
bool m_threadCreationFailed;
bool m_quit;
std::condition_variable m_quitCv;
bool m_waitIdle;
std::condition_variable m_idleCv;
void_task m_idleCallback;

static
void threadProxy( thread_pool *tp );
void theThread();
};

/* constructor with timeout:
allows to set a timeout for the threads until they finish to
wait for new task-queue-items; this allows the pool to satisfy
bursts of requests wit constantly available threads, but it
this bursts end the threads quit after while
*/

// may throw std::system_error

template<typename Rep, typename Period>
thread_pool::thread_pool( unsigned maxThreads,
std::chrono::duration<Rep, Period> const &timeout ) :
m_stop( 0 ),
m_nThreads( 0 ),
m_nThreadsRunning( 0 ),
m_firstQueueId( 0 ),
m_threadCreationFailed( false ),
m_quit( false ),
m_waitIdle( false )
{
using namespace std;
using namespace chrono;
if( maxThreads == 0 )
maxThreads = std::thread::hardware_concurrency(),
maxThreads += !maxThreads;
m_maxThreads = maxThreads;
milliseconds msTimeout = duration_cast<milliseconds>( timeout );
if( timeout > milliseconds( 0 ) )
m_timeout = msTimeout;
else
m_timeout = milliseconds( -1 );

}

// may throw std::system_error
// may throw std::bad_alloc

template<typename F, typename ... Args>
std::uint64_t thread_pool::enqueue_task( F &&f, Args &&...args )
{
using namespace std;
unique_lock<mutex> lock( m_mtx );
uint64_t queueId = m_firstQueueId + m_taskQueue.size();
// emplace a shared-ptr to the packaged task;
// move its task-function as well as the parameters if approppriate
m_taskQueue.emplace_back( bind( forward<F>( f ), forward<Args>( args )
... ) );
// we've less threads than allowed and those which are not running
// are gonna occupied by the thread-queue items ?
if( m_nThreads < m_maxThreads && m_nThreads - m_nThreadsRunning <=
m_taskQueue.size() )
/* except when there's actually no thread created we will update
the thread-counter, release the lock and try to create the thread
then, meaning that the thread-counter could be spurious;
this means that there could be actually less threads created
that than m_nThreads says and other threads see no necessity
to create additional threads before we catch the creation
-exception; so the queue-entities might be processed by less
threads in this situation;
this all is to don't block the queue while creating the thread,
i.e. holding the lock as shortly as possible and allow others
to proceed their enqueues and dequeues */
try
{
if( m_nThreads++ )
lock.unlock();
thread( threadProxy, this ).detach();
}
catch( ... )
{
if( !lock.owns_lock() )
lock.lock();
m_threadCreationFailed = true;
/* if the stop-counter is set, decremement it because
we've inaccurately announced another thread that
actually couldn't be created */
m_stop -= (bool)m_stop;
/* if this was the first attempt to create a thread for
this pool, dequeue the queue-item and rethrow the
exception */
if( !--m_nThreads )
{
/* another thread-creation might have succeeded meanwhile,
the thread might have eaten up our work-item, and all
threads have been stopped meanwhile; so only remove the
work-item if that not happened; this means: check if the
queue-id is valid */
if( (int64_t)(queueId - m_firstQueueId) >= 0 )
m_taskQueue.erase( m_taskQueue.begin() + (size_t)(queueId -
m_firstQueueId) );
throw;
}
}
m_queueCv.notify_one();
return queueId;
}

// set timeout like with the constructor

template<typename Rep, typename Period>
void thread_pool::set_timeout( std::chrono::duration<Rep, Period> const
&timeout )
{
using namespace std;
using namespace chrono;
milliseconds msTimeout = duration_cast<milliseconds>( timeout );
lock_guard<mutex> lock( m_mtx );
if( timeout > milliseconds( 0 ) )
m_timeout = msTimeout;
else
m_timeout = milliseconds( -1 );
}

inline
bool thread_pool::fail()
{
return m_threadCreationFailed;
}

inline
void thread_pool::clear_fail()
{
m_threadCreationFailed = false;
}

template<typename F, typename ... Args>
void thread_pool::idle_callback( F &&f, Args &&...args )
{
using namespace std;
lock_guard<mutex> lock( m_mtx );
m_idleCallback = void_task( bind( forward<F>( f ), forward<Args>( args
) ... ) );
}

// thread_pool.cpp

#include <stdexcept>
#include "thread_pool.h"

// may throw std::system_error

thread_pool::thread_pool( unsigned maxThreads ) :
m_timeout( std::chrono::milliseconds( -1 ) ),
m_stop( 0 ),
m_nThreads( 0 ),
m_nThreadsRunning( 0 ),
m_firstQueueId( 0 ),
m_threadCreationFailed( false ),
m_quit( false ),
m_waitIdle( false )
{
using namespace std;
if( maxThreads == 0 )
maxThreads = std::thread::hardware_concurrency(),
maxThreads += !maxThreads;
m_maxThreads = maxThreads;
}

thread_pool::~thread_pool()
{
using namespace std;
unique_lock<mutex> lock( m_mtx );
while( m_nThreads )
m_quit = true,
m_queueCv.notify_all(),
m_quitCv.wait( lock );
}

// may throw system_error

bool thread_pool::cancel( std::uint64_t queueId )
{
using namespace std;
unique_lock<mutex> lock( m_mtx );
int64_t queueRel = (int64_t)(queueId - m_firstQueueId);
// don't think m_firstQueueId could have been wrapped
// around the point where m_firstQueueId is; its 64 bits
if( queueRel < 0 )
return false;
if( (uint64_t)queueRel >= m_taskQueue.size() )
throw invalid_argument( "thread_pool::cancel - queue-id beyond last
queue-element" );
m_taskQueue.erase( m_taskQueue.begin() + (size_t)queueRel );
return true;
}

// may throw system_error

void thread_pool::wait_idle()
{
using namespace std;
unique_lock<mutex> lock( m_mtx );
// checking m_taskQueue.size() because there all
// be no threads that currently accepted a task becuase
// they didn't get their timeslice yet, so checking
// m_nThreadsRunning alone woudln't be sufficient
while( m_nThreadsRunning || m_taskQueue.size() )
m_waitIdle = true,
m_idleCv.wait( lock );
}

// may throw system_error

void thread_pool::resize( unsigned maxThreads )
{
using namespace std;
if( maxThreads == 0 )
maxThreads = thread::hardware_concurrency(),
maxThreads += !maxThreads;
lock_guard<mutex> lock( m_mtx );
if( maxThreads >= m_nThreads )
// thread-pool growing:
// cancel all thread-stops
m_stop = 0,
m_maxThreads = maxThreads;
else
{
// stop is the number of threads which are
// more than maxThreads
unsigned stop = m_nThreads - maxThreads;
// set the stop-counter which will be decremented by a
// pool-thread
m_stop = stop;
m_maxThreads = maxThreads;
// notifiy the threads to be stopped
if( maxThreads )
for( ; stop; m_queueCv.notify_one(), --stop );
else
m_queueCv.notify_all();
}
}

// may throw system_error

thread_pool::task_queue thread_pool::clear_queue()
{
using namespace std;
lock_guard<mutex> lock( m_mtx );
task_queue tq = move( m_taskQueue );
// advance the queu-id-counter beyond
// all elements we'Ve extracted
m_firstQueueId += tq.size();
return tq;
}

void thread_pool::theThread()
{
using namespace std;
using namespace chrono;
using hrc_tp = time_point<high_resolution_clock>;
auto byeBye = [this]()
{
// one less thread
--m_nThreads;
// the destructor is waiting for us
if( m_quit && !m_nThreads )
// notify the destructor
m_quitCv.notify_one();
// if theres a stop-request for a
// thread, decrement the stop-counter
m_stop -= (bool)m_stop;
};
unique_lock<mutex> lock( m_mtx );
hrc_tp sleepUntil;
for( ; ; )
{
void_task task;
if( m_timeout > milliseconds( 0 ) )
sleepUntil = high_resolution_clock::now() + m_timeout;
for( ; ; )
{
if( m_taskQueue.size() )
{
// there's a task for us:
// dequeue it ...
task = move( m_taskQueue.front() );
m_taskQueue.pop_front();
// advance the queue-id-counter, meaning
// that that task couldn't be cancelled
++m_firstQueueId;
break;
}
if( !m_nThreadsRunning )
{
if( m_waitIdle )
m_waitIdle = false,
m_idleCv.notify_all();
if( m_idleCallback.valid() )
m_idleCallback(),
m_idleCallback.reset();
}
if( m_quit || m_stop )
return byeBye();
if( m_timeout <= milliseconds( 0 ) )
m_queueCv.wait( lock );
else
if( m_queueCv.wait_until( lock, sleepUntil ) == cv_status::timeout )
return byeBye();
}
// we're not idle anymore
++m_nThreadsRunning;
// task will be proceeded without holding the lock
lock.unlock();
// execute the task, catching exceptions when NDEBUG isn'tdefined
try_debug
{
task();
}
catch_debug
{
assert(false);
}
// lock the queue and register us as idling
lock.lock();
--m_nThreadsRunning;

}
}

void thread_pool::threadProxy( thread_pool *tp )
{
tp->theThread();
}

std::size_t thread_pool::to_process()
{
using namespace std;
unique_lock<mutex> lock( m_mtx );
return m_taskQueue.size() + m_nThreadsRunning;
}

void thread_pool::reset_callbacks()
{
using namespace std;
lock_guard<mutex> lock( m_mtx );
m_idleCallback = void_task();
}


Alf P. Steinbach

unread,
Dec 13, 2020, 3:14:49 PM12/13/20
to
One of the most common kinds of bug in Java/C# is calling a derived
class method or during initialization, before the derived class' class
invariant (the basic assumptions about the state) has been established.

That bug is almost non-existent in C++, due to the behavior described
above. That absence of a whole kind of problem is IMO very useful.

- Alf

Bonita Montero

unread,
Dec 13, 2020, 3:20:13 PM12/13/20
to
> While formally correct I never found a practical use case for this
> behavior.

Doesn't matter. As you can see from my experience the whole magic
is optimized away by the compiler (needing optimizations spanning
the translation-units, i.e. link-time code-generation) when it
thinks there won't be any virtual calls in the destructor-chain.
Someone may consider this a bug because there might be virtual
method calls from different threads.

Chris M. Thomasson

unread,
Dec 13, 2020, 4:50:38 PM12/13/20
to
Well... If you invoke a thread in a ctor, and this thread operates on
the object... Well... The thread can start doing things _before_ the
object is fully initialized. It is a common error wrt creating thread
base classes. Try not launch threads in the ctor, that act on the object
being constructed in the first place!

Chris M. Thomasson

unread,
Dec 13, 2020, 4:55:03 PM12/13/20
to
Try to emulate in Relacy.

Chris Vine

unread,
Dec 13, 2020, 8:07:41 PM12/13/20
to
There is no real difference in the requirements for proper sequencing
and (where appropriate) thread synchronization of data objects as
regards code in the body of a constructor and code in the body of any
other method of a class. By the time the constructor is entered all
member sub-objects will have been initialized to their initial state,
so far as they have one (default initialized trivial types may not have
one), including by any member initializer list.

The problem is with calling virtual functions in a base constructor.
This is only really acceptable if you suppress the virtual call by
using an explicit base class scope operator.

Chris M. Thomasson

unread,
Dec 13, 2020, 8:26:20 PM12/13/20
to
I have seen many problems with a ctor of a derived class from a thread
object base thing, starting the thread that calls into the derived
class. There is a race condition. The thread can call into the derived
class before the ctor had a chance to complete.

<pseudo-code>


struct thread_base
{
virtual void enter() = 0;

void create()
{
// create a thread that calls
// raw_thread_entry with a pointer to this
}

static void raw_thread_entry(thread_base* b)
{
b->enter();
}
};


struct derived : public thread_base
{
derived()
{
// some stuff
create();
// some more stuff...
}

void enter()
{
// ummm, this can be called before the ctor completes!
}
};


Iirc, it went something like that. Then there was the problem that
thread_base::raw_thread_entry was not pure C, or something. Technically,
undefined behavior with pthreads. It been a while.

Bonita Montero

unread,
Dec 14, 2020, 1:10:35 AM12/14/20
to
> Try to emulate in Relacy.

Not necessary, it's already tested.

Marcel Mueller

unread,
Dec 14, 2020, 2:45:44 AM12/14/20
to
Am 13.12.20 um 21:14 schrieb Alf P. Steinbach:
> On 13.12.2020 14:35, Marcel Mueller wrote:
>> Yes that is the way C++ initializes and destroys the objects.
>>
>> While formally correct I never found a practical use case for this
>> behavior.
>> In contrast it creates the need for initialization or deinitialization
>> functions that need to be called manually for every class instance. So
>> it is correct but useless. From my point of view a language must be
>> first of all useful.
>
> One of the most common kinds of bug in Java/C# is calling a derived
> class method or during initialization, before the derived class' class
> invariant (the basic assumptions about the state) has been established.

I can't confirm this. In the last two decades I never remember that I or
one of my co-workers run into such a bug in Java or C# (which shares the
same behavior).
Of course, you can run into undefined behavior. But you can do this as
well when calling /any/ non-virtual member function from a constructor
or destructor.

> That bug is almost non-existent in C++, due to the behavior described
> above. That absence of a whole kind of problem is IMO very useful.

Unfortunately the behavior prevents some very useful use cases, i.e.
extending the construction or destruction behavior by a sub class.
A work around is repeated code for an (un)init call in the deepest sub
class constructor or destructor and only there. This is much more error
prone. You need always a second protected constructor or a boolean
argument for further sub classes.
Or you have to push the problem to the application code, which is even
worse, especially in case of the destructor.


Marcel

Bonita Montero

unread,
Dec 14, 2020, 3:58:31 AM12/14/20
to
> One of the most common kinds of bug in Java/C# is calling a derived
> class method or during initialization, before the derived class' class
> invariant (the basic assumptions about the state) has been established.

Is that really a bug but not just a specified different behaviour
than in C++ ?

Chris Vine

unread,
Dec 14, 2020, 5:56:12 AM12/14/20
to
On Sun, 13 Dec 2020 17:25:59 -0800
[snip]
Sure, as I said, the problem is with virtual functions.

In other words, I think your "Try not launch threads in the ctor, that
act on the object being constructed in the first place!" over-eggs the
pudding. Subject to the normal rules on sequencing and synchronization,
there is no problem with accessing member data of the object under
construction because the member data are intialized before the
constructor is entered. The problem is with calling virtual functions
in constructors except through a scoping operator, irrespective of
whether you are launching threads or doing anything else.

Bonita Montero

unread,
Dec 14, 2020, 8:45:48 AM12/14/20
to
> template<typename Rep, typename Period>
> void thread_pool::set_timeout( std::chrono::duration<Rep, Period> const
> &timeout )
> {
>     using namespace std;
>     using namespace chrono;
>     milliseconds      msTimeout = duration_cast<milliseconds>( timeout );
>     lock_guard<mutex> lock( m_mtx );
>     if( timeout > milliseconds( 0 ) )
>         m_timeout = msTimeout;
>     else
>         m_timeout = milliseconds( -1 );
m_queueCv.notify_all();
> }

Otherwise the threads will begin their waiting-period only
after they've processed a queue-entity.

Chris M. Thomasson

unread,
Dec 14, 2020, 4:45:35 PM12/14/20
to
On 12/13/2020 10:10 PM, Bonita Montero wrote:
>> Try to emulate in Relacy.
>
> Not necessary, it's already tested.

Well, it does not hurt to emulate it in a race detector, just for fun?

Chris M. Thomasson

unread,
Dec 14, 2020, 6:18:17 PM12/14/20
to
I was basically referring to the stuff in the ctor:

>> derived()
>> {
>> // some stuff
>> create();
>> // some more stuff...
>> }

create(); can operate on the object before "some more stuff..." is
committed. Wrt threads, sometimes the thread will start after its
finished, and all is well... Other times it can run right then and
there, and operate on the object before "some more stuff..." had a
chance to run, or right in the middle of it. I have seen code like this
in the past.


The problem is with calling virtual functions
> in constructors except through a scoping operator, irrespective of
> whether you are launching threads or doing anything else.
>

Basically, I like to create the object fully, then run the thread. Only
run threads after all ctors are finished.

Melzzzzz

unread,
Dec 14, 2020, 7:37:16 PM12/14/20
to
Joinining in destructor was always no no if Java model is used.
Not once I saw pworblems with this in RL.


--
current job title: senior software engineer
skills: c++,c,rust,go,nim,haskell...

press any key to continue or any other to quit...
U ničemu ja ne uživam kao u svom statusu INVALIDA -- Zli Zec
Svi smo svedoci - oko 3 godine intenzivne propagande je dovoljno da jedan narod poludi -- Zli Zec
Na divljem zapadu i nije bilo tako puno nasilja, upravo zato jer su svi
bili naoruzani. -- Mladen Gogala

Chris Vine

unread,
Dec 14, 2020, 8:00:55 PM12/14/20
to
On Mon, 14 Dec 2020 15:17:55 -0800
Then you have incorrect synchronization. I have seen lots of defective
code arising from incorrect synchronization, but in your example that
has nothing to do with whether the code happens to be running in a
constructor or not; it would equally be a problem if any incorrectly
synchronized code in derived() were instead contained in an ordinary
member function which started a thread. However, there _is_ a problem
with the constructor, which is that it calls thread_base::create(),
which via thread_base::raw_thread_entry() calls enter() which is a
virtual function.

red floyd

unread,
Dec 14, 2020, 11:57:23 PM12/14/20
to
Even if you declare it pure virtual, you still need to provide an
implementation for this very scenario.

e.g:

class Bonita {
virtual ~Bonita() = 0; // or is it nullptr here?
};

Bonita::~Bonita() { }

Chris M. Thomasson

unread,
Dec 15, 2020, 12:53:55 AM12/15/20
to
Yeah. your right. I have seen a lot of broken code through the years
that has this race condition, to the point where I thought it was a
common mistake. Would cringe every time I would see a thread base class.

Juha Nieminen

unread,
Dec 15, 2020, 2:11:22 AM12/15/20
to
Paavo Helde <myfir...@osa.pri.ee> wrote:
>> I don't think std::shared_ptr would have remedied that in any way.
>> When the last std::shared_ptr object is destroyed, it will destroy the
>> managed object, and the problem will happen. std::shared_ptr doesn't
>> somehow magically guard against that problem.
>
> Yes it does. When you call a member function of an object via
> std::shared_ptr then it means its destructor has not yet started
> (neither in this nor in another thread), so there is no danger to call a
> pure virtual method.

You are saying that "if you call the member function from anywhere else than
the destructor, it will work fine". I still don't see how std::shared_ptr
enters the picture in this.

Juha Nieminen

unread,
Dec 15, 2020, 2:13:53 AM12/15/20
to
Bonita Montero <Bonita....@gmail.com> wrote:
> Here's the current state of my thread-pool-class:

If you want people to help you with a programming problem, you should
make an absolutely minimal piece of code that reproduces the problem,
rather than expect people to wade through 400+ lines of cryptic code
for you.

Chris M. Thomasson

unread,
Dec 15, 2020, 2:22:25 AM12/15/20
to
This has been tested?

Bonita Montero

unread,
Dec 15, 2020, 3:05:22 AM12/15/20
to
>> Here's the current state of my thread-pool-class:

> If you want people to help you with a programming problem, you should
> make an absolutely minimal piece of code that reproduces the problem,
> rather than expect people to wade through 400+ lines of cryptic code
> for you.

My code is not cryptic.
The parts that are not easy to understand are documented.

Bonita Montero

unread,
Dec 15, 2020, 3:12:13 AM12/15/20
to
>> Otherwise the threads will begin their waiting-period only
>> after they've processed a queue-entity.

> This has been tested?

It doesn't need to be tested since it's trivial.
The non-trivial parts are tested.

Bonita Montero

unread,
Dec 15, 2020, 3:13:23 AM12/15/20
to
>>> Try to emulate in Relacy.

>> Not necessary, it's already tested.

> Well, it does not hurt to emulate it in a race detector, just for fun?

That's not necessary.
I've used it in a program with tousands of queue-items and with
as much threads as there are hw-threads in my computer (16).

Chris M. Thomasson

unread,
Dec 15, 2020, 3:21:38 AM12/15/20
to
Well, that's not good enough. Sorry.

Chris M. Thomasson

unread,
Dec 15, 2020, 3:24:15 AM12/15/20
to
On 12/15/2020 12:11 AM, Bonita Montero wrote:
>>> Otherwise the threads will begin their waiting-period only
>>> after they've processed a queue-entity.
>
>> This has been tested?
>
> It doesn't need to be tested since it's trivial.

Well, you did make a "correction", right? You snipped it. Btw, can you
please use proper quoting?


> The non-trivial parts are tested.

How did you test them?


Chris M. Thomasson

unread,
Dec 15, 2020, 3:58:12 AM12/15/20
to
On 12/13/2020 9:55 AM, Bonita Montero wrote:
> Here's the current state of my thread-pool-class:
[...]
> private:
>     std::mutex                m_mtx;
>     unsigned                  m_maxThreads;
>     std::chrono::milliseconds m_timeout;
>     unsigned                  m_stop;
>     unsigned                  m_nThreads;
[...]
> void thread_pool::theThread()
> {
>     using namespace std;
>     using namespace chrono;
>     using hrc_tp = time_point<high_resolution_clock>;
>     auto byeBye = [this]()
>     {
>         // one less thread
>         --m_nThreads;
[...]

Is m_nThreads allowed to be accessed by multiple threads?

Chris M. Thomasson

unread,
Dec 15, 2020, 4:02:45 AM12/15/20
to
On 12/13/2020 9:55 AM, Bonita Montero wrote:
> Here's the current state of my thread-pool-class:
>
> // debug_exceptions.h
[...]
>         try
>         {
>             if( m_nThreads++ )
>                 lock.unlock();
>             thread( threadProxy, this ).detach();
>         }
[...]
Are you creating a new thread per-task?

Paavo Helde

unread,
Dec 15, 2020, 4:18:01 AM12/15/20
to
This thread is about calling member functions of an object from another
thread while the destructor of the object has already started in the
first thread.

It's very simple. For avoiding to call a member function from another
thread while the destructor of the object has started in the first
thread, one needs synchronization. std::shared_ptr is designed to
provide that synchronization, and it's in the standard, so it ought to
be the preferred method to use.

One can use other ways of synchronization, but that would be more
complicated and more error-prone (as demonstrated by the first post in
this thread - it had synchronization, but in the wrong place).

Fred. Zwarts

unread,
Dec 15, 2020, 4:58:20 AM12/15/20
to
Op 15.dec..2020 om 10:17 schreef Paavo Helde:
I understand that shared_ptr can be a good synchronization method, but
shared_ptr uses a delete on the pointer when the last shared_ptr is
destructed. So, your premise is that a delete can be done, i.e. that the
class was created dynamically. Not all classes used by threads are
created dynamically, so other synchronization methods are sometimes
preferable.

Bonita Montero

unread,
Dec 15, 2020, 5:22:28 AM12/15/20
to
>> That's not necessary.
>> I've used it in a program with tousands of queue-items and with
>> as much threads as there are hw-threads in my computer (16).

> Well, that's not good enough. Sorry.

That's good enough because the synchronization-parts are trivial.

Bonita Montero

unread,
Dec 15, 2020, 5:23:25 AM12/15/20
to
> Is m_nThreads allowed to be accessed by multiple threads?

Yes, but only if the mutex is locked.
Sorry, but you seem to be too stupid to read < 250 lines of
trivial code.

Bonita Montero

unread,
Dec 15, 2020, 5:25:48 AM12/15/20
to
Look here:

// we've less threads than allowed and those which are not running
// are gonna occupied by the thread-queue items ? if(
m_nThreads < m_maxThreads && m_nThreads - m_nThreadsRunning <=
m_taskQueue.size() )

The code is documented and you can't read it.
Again: this checks if there are less threads than m_maxThreads and the
threads not currently active might be all occupied by not accepted queue
-items.

Bonita Montero

unread,
Dec 15, 2020, 5:39:28 AM12/15/20
to
> Even if you declare it pure virtual, you still need to provide an
> implementation for this very scenario.
> e.g:
> class Bonita {
>    virtual ~Bonita() = 0; // or is it nullptr here?
> };
> Bonita::~Bonita() { }

That's true for a pure-virtual destructor because it is called
for every object which is destructed. But that's not true for
objects whose pure virtual methods aren't called.

Paavo Helde

unread,
Dec 15, 2020, 6:56:25 AM12/15/20
to
Right, and these methods are more complicated and more error-prone.

Let's see in which ways I would have a thread-shared object which is not
allocated dynamically, and there is a danger that another thread might
call its member functions while its destructor has started.

1. It's a global static. When its destructor is started, it means that
the program is about to terminate. Before terminating all other threads
should be already joined, so the situation described by OP cannot happen
at all (or in other words, thread joining provides the needed
synchronization here).

2. It's a local variable in a stack frame. Passing a pointer of a local
stack variable to another thread is pretty fragile, needs complicated
synchronization and is error-prone. I hope you do not argue with that.
In particular, having the synchronization in a base class destructor
cannot work, but nevertheless this approach seems to be invariably
reinvented all the time, by some reason.

3. It's a member variable in another larger object which is allocated
dynamically and shared between the threads. In that case the larger
object should be accessed via a shared_ptr, and the whole issue does not
appear.

Chris Vine

unread,
Dec 15, 2020, 9:38:22 AM12/15/20
to
On Mon, 14 Dec 2020 21:53:34 -0800
"Chris M. Thomasson" <chris.m.t...@gmail.com> wrote:
> On 12/14/2020 5:00 PM, Chris Vine wrote:
[snip]
> > However, there _is_ a problem
> > with the constructor, which is that it calls thread_base::create(),
> > which via thread_base::raw_thread_entry() calls enter() which is a
> > virtual function.
>
> Yeah. your right. I have seen a lot of broken code through the years
> that has this race condition, to the point where I thought it was a
> common mistake. Would cringe every time I would see a thread base class.

Hopefully this particular wrongness is becoming a thing of the past.
In the days of the mad OOP craze you had virtual functions everywhere,
including in thread base classes. A common idiom was to have a base
thread class with a pure virtual function called "start" or some
similar name, which you would override by inheritance with the concrete
implementation of the code to be executed by the thread in question.
Nowadays you just pass in a lambda function or std::function object
containing the implementation to your thread class, which obviates all
that: std::thread is not intended to be derived from.

Bonita Montero

unread,
Dec 15, 2020, 10:08:04 AM12/15/20
to
> This thread is about calling member functions of an object from another
> thread while the destructor of the object has already started in the
> first thread.

shared_ptr wouldn't help here because I've got a thread which is running
until a destructor waits for its termination and this thread might call
a virtual function before the thread waits for its end.

> It's very simple. For avoiding to call a member function from another
> thread while the destructor of the object has started in the first
> thread, one needs synchronization. std::shared_ptr is designed to
> provide that synchronization, and it's in the standard, so it ought
> to be the preferred method to use.
> One can use other ways of synchronization, but that would be more
> complicated and more error-prone (as demonstrated by the first post
> in this thread - it had synchronization, but in the wrong place).

shared_ptr<> is about holding multiple references to the same object
from different threads. Having multiple copies of the shared_ptr<>
in one thread is not what it is thought for because copying a shared
_ptr<> is slow. Within a thread you simply extract normal pointers
of a shared_ptr<>-object which live shorter than the shared_ptr<>
object itself.

Paavo Helde

unread,
Dec 15, 2020, 10:18:54 AM12/15/20
to
15.12.2020 17:07 Bonita Montero kirjutas:
>> This thread is about calling member functions of an object from
>> another thread while the destructor of the object has already started
>> in the first thread.
>
> shared_ptr wouldn't help here because I've got a thread which is running
> until a destructor waits for its termination and this thread might call
> a virtual function before the thread waits for its end.

shared_ptr would help, because if you accessed the object via a
shared_ptr in both threads, the destructor would not be started while
there is a shared_ptr to the object alive, in any thread.

This would have forced you to rethink the design and place the
synchronization elsewhere, not in the base class destructor. Problem
avoided.

Bonita Montero

unread,
Dec 15, 2020, 10:21:58 AM12/15/20
to
> shared_ptr would help, because if you accessed the object via a
> shared_ptr in both threads, the destructor would not be started
> while there is a shared_ptr to the object alive, in any thread.

If the thread would be the last to have a shared_ptr<> it would call
the destructor which would wait for the termination of the thread.
So this wouldn't work.

Scott Lurndal

unread,
Dec 15, 2020, 12:03:33 PM12/15/20
to
Melzzzzz <Melz...@zzzzz.com> writes:
>On 2020-12-13, Bonita Montero <Bonita....@gmail.com> wrote:
>> I had a nice bug: I've got a base-class with a pure virtual function
>> which is overridden in a derived class. This function is virtually
>> called by a thread from a thread pool (through a static proxy-func-
>> tion). In the destructor of the base-class I wait for all threads
>> from the thread pool to have finished their work and that there are
>> no more items in their work-queue.
>> What I missed was, that as soon as the base-class destructor is called
>> the virtual fuction pointer is adjusted to the pointer of the base-class
>> (I'm wondering that is there any because a base-class with a pure vir-
>> tual function shouldn't have a virtual method table at all). So before
>> I could wait for the threads to finish all work the null pure virtual
>> function got called. ;-)
>> Interestingly this doesn't happen if I don't compile the code as debug
>> -code. So the compiler indirectly gives me a hint here for a possible
>> bug.
>
>Joinining in destructor was always no no if Java model is used.
>Not once I saw pworblems with this in RL.

bool in_context(void) const { return t_thread == pthread_self(); }

c_thread::~c_thread(void)
{
lock();
terminate();
unlock();
}

/**
* Terminate the thread. If called in-context, it will clear the
* running flag and exit. If called out of context, it will cancel
* the thread and join with it.
*/
void
c_thread::terminate(void)
{
if (in_context()) {
t_running = false;
pthread_exit(NULL);
} else {
int diag = pthread_cancel(t_thread);
if (diag == ESRCH) {
/* Thread already gone, nothing to do */
} else if (diag != 0) {
t_logger->log("%s Unable to cancel thread: %s\n",
t_thread_name, strerror(diag));
}

void *rval;
diag = pthread_join(t_thread, &rval);
if (diag == ESRCH) {
/* Thread already gone, nothing to do */
} else if (diag != 0) {
t_logger->log("%s Thread join failed: %s\n",
t_thread_name, strerror(diag));
}
t_running = false;
}

Chris M. Thomasson

unread,
Dec 15, 2020, 4:48:32 PM12/15/20
to
____________________
void thread_pool::theThread()
{
using namespace std;
using namespace chrono;
using hrc_tp = time_point<high_resolution_clock>;
auto byeBye = [this]()
{
// one less thread
--m_nThreads;
___________________

Where is the the mutex locked here? What did I miss?

Chris M. Thomasson

unread,
Dec 15, 2020, 4:50:38 PM12/15/20
to
On 12/15/2020 2:23 AM, Bonita Montero wrote:
I only had time to briefly skim it.

Chris M. Thomasson

unread,
Dec 15, 2020, 5:06:00 PM12/15/20
to
Ahhh. I see, you are calling byeBye() under the protection of the lock
further down. Its was a little confusing. Sorry.

Chris M. Thomasson

unread,
Dec 15, 2020, 5:13:15 PM12/15/20
to
I have heard the same argument for ages.

red floyd

unread,
Dec 15, 2020, 7:12:48 PM12/15/20
to
I misread your original. I thought you said you had a pure virtual
destructor.

Bonita Montero

unread,
Dec 16, 2020, 1:54:23 AM12/16/20
to
> ____________________
> void thread_pool::theThread()
> {
>      using namespace std;
>      using namespace chrono;
>      using hrc_tp = time_point<high_resolution_clock>;
>      auto byeBye = [this]()
>      {
>          // one less thread
>          --m_nThreads;
> ___________________
>
> Where is the the mutex locked here? What did I miss?

Before byeBye is called.

Juha Nieminen

unread,
Dec 16, 2020, 2:05:37 AM12/16/20
to
Paavo Helde <myfir...@osa.pri.ee> wrote:
> 15.12.2020 09:11 Juha Nieminen kirjutas:
>> Paavo Helde <myfir...@osa.pri.ee> wrote:
>>>> I don't think std::shared_ptr would have remedied that in any way.
>>>> When the last std::shared_ptr object is destroyed, it will destroy the
>>>> managed object, and the problem will happen. std::shared_ptr doesn't
>>>> somehow magically guard against that problem.
>>>
>>> Yes it does. When you call a member function of an object via
>>> std::shared_ptr then it means its destructor has not yet started
>>> (neither in this nor in another thread), so there is no danger to call a
>>> pure virtual method.
>>
>> You are saying that "if you call the member function from anywhere else than
>> the destructor, it will work fine". I still don't see how std::shared_ptr
>> enters the picture in this.
>
> This thread is about calling member functions of an object from another
> thread while the destructor of the object has already started in the
> first thread.

I see now.

So essentially mutual exclusion is needed to guard against the object
being destroyed while another thread is calling some member function of it,
and the solution you suggest is that every thread only accesses the
object through a shared_ptr, which guarantees that the object will not
be destroyed until the last shared_ptr pointing to it is destroyed
(and that two threads destroying their own shared_ptr instance pointing
to that object at the same time is thread-safe.)

Juha Nieminen

unread,
Dec 16, 2020, 2:10:06 AM12/16/20
to
Bonita Montero <Bonita....@gmail.com> wrote:
> shared_ptr<> is about holding multiple references to the same object
> from different threads. Having multiple copies of the shared_ptr<>
> in one thread is not what it is thought for because copying a shared
> _ptr<> is slow. Within a thread you simply extract normal pointers
> of a shared_ptr<>-object which live shorter than the shared_ptr<>
> object itself.

It sounds like you need mutual exclusion on the object anyway, so whatever
you do it's going to be slower than a mere pointer assignment.

The only exception I can think of is if, within the same thread, you keep
a shared_ptr instance pointing to the managed object and then use raw
pointers pointing to it elsewhere within the thread, making absolutely
sure that no such raw pointer is accessed after the shared_ptr has been
destroyed.

Juha Nieminen

unread,
Dec 16, 2020, 2:12:03 AM12/16/20
to
Bonita Montero <Bonita....@gmail.com> wrote:
>>> Here's the current state of my thread-pool-class:
>
>> If you want people to help you with a programming problem, you should
>> make an absolutely minimal piece of code that reproduces the problem,
>> rather than expect people to wade through 400+ lines of cryptic code
>> for you.
>
> My code is not cryptic.
> The parts that are not easy to understand are documented.

But copy-pasting 400+ lines of code without reducing it to the absolute
minimum that reproduces the problem shows that you didn't do any work
to make the problem easier to discern, and are expecting others to do
that work for you.

Bonita Montero

unread,
Dec 16, 2020, 2:14:45 AM12/16/20
to
> It sounds like you need mutual exclusion on the object anyway, so
> whatever you do it's going to be slower than a mere pointer assignment.

I was discussing shared_ptr<> in general but not in relation to
what I did.

> The only exception I can think of is if, within the same thread, you keep
> a shared_ptr instance pointing to the managed object and then use raw
> pointers pointing to it elsewhere within the thread, making absolutely
> sure that no such raw pointer is accessed after the shared_ptr has been
> destroyed.

Then use unique_ptr<>.

Bonita Montero

unread,
Dec 16, 2020, 2:15:52 AM12/16/20
to
>> My code is not cryptic.
>> The parts that are not easy to understand are documented.

> But copy-pasting 400+ lines of code without reducing it to the absolute
> minimum that reproduces the problem shows that you didn't do any work
> to make the problem easier to discern, and are expecting others to do
> that work for you.

The code doesn't show my problem.
I just wanted to show an elegant implementation of a thread-pool.

Paavo Helde

unread,
Dec 16, 2020, 3:03:39 AM12/16/20
to
Right. The desired goal is that as now the destructor would not be
reached while any shared pointer to the object is alive, Bonita would be
forced to move her thread synchronization code out of there and place it
in a more appropriate place.

Bonita Montero

unread,
Dec 16, 2020, 3:27:09 AM12/16/20
to
> Right. The desired goal is that as now the destructor would not be
> reached while any shared pointer to the object is alive, Bonita would be
> forced to move her thread synchronization code out of there and place it
> in a more appropriate place.

The thread-pool-objects are destructed after the destructor of the con-
taining object is called. The issue here is that the threads indirectly
call a virtual method of the containing object while destrucor waits
for their termination in the destructor and the VMT-pointer has been
adjusted to null. So the simplest solution was to use a function pointer
instead of a virtual function and everything works as intended now.

Richard Damon

unread,
Dec 16, 2020, 7:32:41 AM12/16/20
to
Except tha with the modified design, the destructor wouldn't be waiting
for the termination of the threads, because it would know that all the
threads were ended or are in the process of ending.

Bonita Montero

unread,
Dec 16, 2020, 8:03:01 AM12/16/20
to
Sorry, that's stupid. I need a proper cleanup since the threads will
usually terminate before the process ends.

Chris M. Thomasson

unread,
Dec 16, 2020, 8:02:12 PM12/16/20
to
Using detached threads can be a bit sketchy. I never really found a need
to actually use them.

Bonita Montero

unread,
Dec 17, 2020, 1:22:54 AM12/17/20
to

>> Sorry, that's stupid. I need a proper cleanup since the threads will
>> usually terminate before the process ends.

> Using detached threads can be a bit sketchy. I never really found a need
> to actually use them.

I don't need the thread-objects after creation.

Bonita Montero

unread,
Dec 17, 2020, 6:45:15 AM12/17/20
to
>> It doesn't need to be tested since it's trivial.

> Well, you did make a "correction", right? You snipped it. Btw, can you
> please use proper quoting?

That was a minor bug. It wouldn't cause any malfunctions but only
that a threads might release its resources when it times out.

>> The non-trivial parts are tested.

> How did you test them?

With code that uses this thread-pool.

Bonita Montero

unread,
Dec 17, 2020, 6:46:26 AM12/17/20
to
>>> Well, that's not good enough. Sorry.

>> That's good enough because the synchronization-parts are trivial.

> I have heard the same argument for ages.

I'm not using any freaky Lock-free synchronization
but only standard parts.

Chris M. Thomasson

unread,
Dec 17, 2020, 2:45:53 PM12/17/20
to
On 12/15/2020 6:38 AM, Chris Vine wrote:
> On Mon, 14 Dec 2020 21:53:34 -0800
> "Chris M. Thomasson" <chris.m.t...@gmail.com> wrote:
>> On 12/14/2020 5:00 PM, Chris Vine wrote:
> [snip]
>>> However, there _is_ a problem
>>> with the constructor, which is that it calls thread_base::create(),
>>> which via thread_base::raw_thread_entry() calls enter() which is a
>>> virtual function.
>>
>> Yeah. your right. I have seen a lot of broken code through the years
>> that has this race condition, to the point where I thought it was a
>> common mistake. Would cringe every time I would see a thread base class.
>
> Hopefully this particular wrongness is becoming a thing of the past.

Alas, I am not nearly as active as I was around 15-20 years ago wrt
programing threads and creating synchronization algorithms. However, I
share in your hope! :^)

Remember use a lot of assembly language in x86 and SPARC to create
sensitive sync algorithms. Ahhh, the good ol' days when
comp.programming.threads was actually useful. Now, it seems to be a
wasteland. Got to know really smart people on there, like David
Butenhof. I miss conversing with him.

There was the great lock-free debates on c.p.t.


> In the days of the mad OOP craze you had virtual functions everywhere,
> including in thread base classes. A common idiom was to have a base
> thread class with a pure virtual function called "start" or some
> similar name, which you would override by inheritance with the concrete
> implementation of the code to be executed by the thread in question.

Indeed. Pre-C++11, where people were using either POSIX or Windows for
threading. I used a lot of C, but when it came to "wrapping" threads in
C++, the almighty base class was used a lot. Argh!


> Nowadays you just pass in a lambda function or std::function object
> containing the implementation to your thread class, which obviates all
> that: std::thread is not intended to be derived from.

That is a good thing.

Bonita Montero

unread,
Dec 17, 2020, 2:50:08 PM12/17/20
to
> Remember use a lot of assembly language in x86 and SPARC to create
> sensitive sync algorithms. ...

Really ?
Intrinsics for atomic variable accesses have been there for a long time.

Chris M. Thomasson

unread,
Dec 17, 2020, 2:54:08 PM12/17/20
to
Your thread pool is a bit different than the usual. Basically, a
"vanilla" thread pool would simply create one or two threads per CPU,
and those would stay there until pool destruction. To destroy the pool,
one would send a special message that would cause a thread to terminate.
So, send the "death" messages, then join all of the threads...

I remember creating many experiments for the method of a user sending
work to the pool. Kind of a network of distributed queues. It was
layered. Each thread had their own local queues, then there was a bunch
of global queues. I used an "eventcount" thing I made to allow for an
efficient way for a work thread to wait on its local queues _and_ the
global queues, all in one.

Man, I loved that eventcount! Btw, its nothing like a semaphore.

Chris M. Thomasson

unread,
Dec 17, 2020, 2:57:10 PM12/17/20
to
Yeah, but I have seen many people mess up locking as well. One time,
around 18 years ago... One person said all is good... The system has
been running non-stop for around a month. Then, all of a sudden... BAM!
Deadlock. I helped debug it. It was a bitch and a half.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:04:59 PM12/17/20
to
On 12/17/2020 3:44 AM, Bonita Montero wrote:
>>> It doesn't need to be tested since it's trivial.
>
>> Well, you did make a "correction", right? You snipped it. Btw, can you
>> please use proper quoting?
>
> That was a minor bug. It wouldn't cause any malfunctions but only
> that a threads might release its resources when it times out.

Still, it does not hurt to run it through, say, ThreadSanitizer: Just
for fun?


>
>>> The non-trivial parts are tested.
>
>> How did you test them?
>
> With code that uses this thread-pool.
>

Okay.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:08:04 PM12/17/20
to
On 12/17/2020 11:49 AM, Bonita Montero wrote:
> > Remember use a lot of assembly language in x86 and SPARC to create
> > sensitive sync algorithms. ...
>
> Really ?

Really. Big time.

> Intrinsics for atomic variable accesses have been there for a long time.

They were not standard. Using ASM was for piece of mind. Did you ever
hear of an old GCC bug that would break POSIX, well before C++11? It was
a mutex issue. I might try to dig up the old message.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:10:05 PM12/17/20
to
On 12/17/2020 11:49 AM, Bonita Montero wrote:
This was because C did not not know about threading. If it claimed to be
POSIX compliant, okay, but there were still issues with memory barriers
and exotic lock-free and wait-free algorithms. C had no idea about them.

Bonita Montero

unread,
Dec 17, 2020, 3:13:28 PM12/17/20
to
> Your thread pool is a bit different than the usual. Basically, a
> "vanilla" thread pool would simply create one or two threads per CPU,
> and those would stay there until pool destruction. To destroy the pool,
> one would send a special message that would cause a thread to terminate.
> So, send the "death" messages, then join all of the threads...

The number of threads need to be equally to the number of cores to have
a full load if all threads are running; therefore you can set the number
of threads to zero which defaults to the number of hw-threads.
When you need more threads there are two cases. First your threads also
should staturate your CPU, but threads might regulary waiting on events
and leave compution-time to other threads in the pool. Second if you
need an equal share of the computation time among all tasks the pool
theoretically wouldn't have an upper limit and the use of the pool
would be only that you have less costs than with creating individual
threas. In this case my approach with a timeout for the threads is
very useful: the timeout would keep running the threads for a coninous
burst of tasks, but if that burst ends the threads give up their
resources after a while.

Bonita Montero

unread,
Dec 17, 2020, 3:15:28 PM12/17/20
to
>> I'm not using any freaky Lock-free synchronization
>> but only standard parts.

> Yeah, but I have seen many people mess up locking as well. One time,
> around 18 years ago... One person said all is good... The system has
> been running non-stop for around a month. Then, all of a sudden... BAM!
> Deadlock. I helped debug it. It was a bitch and a half.

Where should I have induced a deadlock ?
The version of the thread-pool I've shown lately uses only one mutex.

Bonita Montero

unread,
Dec 17, 2020, 3:18:24 PM12/17/20
to
>> That was a minor bug. It wouldn't cause any malfunctions but only
>> that a threads might release its resources when it times out.

> Still, it does not hurt to run it through, say, ThreadSanitizer: Just
> for fun?
You don't have the concentration to trust in your work as you seem
a bit chaotic; that's not uncommon for people who are interested in
performance-programming because an affinity of thinking at the resource
-level is often bound to mental issues.

Bonita Montero

unread,
Dec 17, 2020, 3:24:27 PM12/17/20
to
> They were not standard. ...

When you are bound to gcc only you could use these intrinsics portably
across different CPUs (__sync_*). And clang supports them compatibly
for a while. MSVC also has atomic intrinsics for a long time. So you
simply could have some #ifdefs to do the small part of the code which
can't be shared among the platforms.
And even with C++11 there could be reasons to use these intrinsics
instead of the C++11-atomics because C++11 doesn't support DCAS.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:51:02 PM12/17/20
to
Iirc, they forgot to unlock a mutex under a very hard to trip condition.
It was a distributed system, so, this one unlocked mutex was just a
ticking time bomb for a deadlock. It was in C with no scoped locking
semantics, no automatic unlocking, it was 100% manual.

Bonita Montero

unread,
Dec 17, 2020, 3:56:44 PM12/17/20
to
>> Where should I have induced a deadlock ?
>> The version of the thread-pool I've shown lately uses only one mutex.

> Iirc, they forgot to unlock a mutex under a very hard to trip condition.
> It was a distributed system, so, this one unlocked mutex was just a
> ticking time bomb for a deadlock. It was in C with no scoped locking
> semantics, no automatic unlocking, it was 100% manual.

What I said was related to what I did and not abou your anecdotes.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:57:51 PM12/17/20
to
You are an odd person. Anyway, imvvho, using ThreadSanitizer is okay.
Using Relacy is alright. No problem. Then there is the opposite
argument. Trusting in your work in one thing... Then seeing how its
subtly broken in a obscure part using a race detector is another. Large
programs can get complex.

Chris M. Thomasson

unread,
Dec 17, 2020, 3:59:39 PM12/17/20
to
You are to terse. I was just trying to have a conversation.

Chris M. Thomasson

unread,
Dec 17, 2020, 4:06:04 PM12/17/20
to
On 12/17/2020 12:24 PM, Bonita Montero wrote:
>> They were not standard. ...
>
> When you are bound to gcc only you could use these intrinsics portably
> across different CPUs (__sync_*).

It was actually easier for me to use GAS to assemble all of my sync code
way back then. It gave me piece of mind. I wrote several versions, in
x86, SPARC and some PPC. Back then, there was a lot of issues for
compiler optimization, or the worst... Link time optimization to break
things. Then the GCC mutex issue, breaking POSIX rules.


> And clang supports them compatibly
> for a while. MSVC also has atomic intrinsics for a long time. So you
> simply could have some #ifdefs to do the small part of the code which
> can't be shared among the platforms.
> And even with C++11 there could be reasons to use these intrinsics
> instead of the C++11-atomics because C++11 doesn't support DCAS.

Well, getting DWCAS on x86 required ASM way back then. Iirc, there was
no intrinsic on it, CMPXCHG8B, 20 years ago. There was a lot of issues
before threading and atomics actually became _standard_, within the
languages themselves, C and C++.

Chris M. Thomasson

unread,
Dec 17, 2020, 10:53:57 PM12/17/20
to
On 12/17/2020 12:24 PM, Bonita Montero wrote:
>> They were not standard. ...
>
> When you are bound to gcc only

I used GCC, but was not bound to it.
0 new messages