remove outdated comment
FIX: AbstractUploader worker thread synchronisation
adapt T_UploadFacility test cases
TEST: regression test for the worker thread destruction race
AbstractUploader::TearDown() in Spooler::TearDown()
remove redundant method Spooler::WaitForTermination()
make sure Spooler objects get torn down gracefully
remove useless method Spooler::TearDown()
ensure that AbstractUploader::TearDown() is called in Spooler's dtor
Merge pull request #336 from reneme/fix_race

FIX: Race Condition between AbstractUploader Initialisation and Destruction

Problem Description

There was a race between the initialisation of the AbstractUploader and the destruction of the resulting instance. Namely it was possible that a worker thread spawned in AbstractUploader::Initialize() would call the pure virtual method AbstractUploader::WorkerThread() after the destructor of a concrete implementation of AbstractUploader has been executed, resulting in a 'pure virtual method called' error.
Additionally it was not certain, that the implementation of AbstractUploader::WorkerThread() would have been terminated before aforementioned destructor would run. That might have caused an invalid this pointer inside the concrete implementation of AbstractUploader::WorkerThread().

Example (Broken Scenario)

AbstractUploader *u = AbstractUploader::Construct(spooler_definition);
u->Upload(...)
delete u;
The delete might have destroyed the ConcreteUploader constructed in AbstractUploader::Construct before a worker thread started in AbstractUploader::Initialize() would have started executing.
See unit test T_UploadFacility.InitDtorRace which runs exactly this scenario.

Fix

Now AbstractUploader::Initialize() returns only after the worker thread started executing, thus there is no way for the user to invoke the instance's destructor prematurely. Additionally there is now AbstractUploader::TearDown() that takes care of the secure termination of the worker thread. This method is meant to be called right before the destruction of the concrete uploader instance. An assert() in AbstractUploader's destructor makes sure that TearDown() was called!

Furthermore

Additionally this Pull Request removes some old garbage from the Spooler class. Namely the method Spooler::WaitForTermination() which does not make sense anymore (and actually did the same as Spooler::WaitForUpload()).
Spooler instances in various places of the code are now removed gracefully after usage.
Finally there is a regression test to check the fixed error condition.