Wait the thread to actually finish before the Thread destructor exits #445
Conversation
…. Also use weak_ptr in ShutdownTask to prevent cyclic reference problem of shared_ptr. fixes hazelcast#429
ihsandemir
changed the title
|
Windows test FAILed. |
1 similar comment
|
Windows test FAILed. |
|
Linux test PASSed. |
1 similar comment
|
Linux test PASSed. |
|
Windows test PASSed. |
|
Linux test PASSed. |
| AbstractThread::~AbstractThread() { | ||
| if (started) { | ||
| int state = stateLatch->get(); | ||
| if (state == 1) { |
There was a problem hiding this comment.
Why is this if statement necessary. It looks like the second count is doing same thing as started boolean. I would remove the second count that tracks started.
And there seems to be still race between destructor and start method.
When both starts at the same time, it could be the case that destructor passes then thread starts.
To prevent that, ( I am asking because I am not sure about usages of abstractThread )
there needs to be a third state and started that we set on destructor so that start will be prevented.
There was a problem hiding this comment.
Changed the Thread implementation to use state machine. The states are :
enum ThreadState {
UNSTARTED = 0,
STARTED = 1,
JOINED = 2,
CANCELLED = 3
};
It can go from UNSTARTED to STARTED, STARTED to JOINED or STARTED to CANCELLED states. Other transitions are not valid. When thread is cancelled, then the shutdown latch is waited.
enum ThreadState {
UNSTARTED = 0,
STARTED = 1,
JOINED = 2,
CANCELLED = 3
};
It can go from UNSTARTED to STARTED, STARTED to JOINED or STARTED to CANCELLED states. Other transitions are not valid. When thread is cancelled, then the shutdown latch is waited.
|
Windows test PASSed. |
|
Linux test PASSed. |
3 participants
Wait the thread to actually finish before the Thread destructor exits. Also use weak_ptr in ShutdownTask to prevent cyclic reference problem of shared_ptr.
fixes #429