Code review comment for lp:~compiz-team/compiz/compiz.fix_1085591

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> Seems to work now. Bug fixed.
>
> Just a note on pthread creation in AsyncTask::AsyncTask... It's usually a bad
> idea to create or touch anything outside of your own class in a constructor.
> Say for example, someone uses your class and decides they need a vector<> of
> them. Every time the vector reallocates, every element gets re-copy-
> constructed and destroyed. In this case, that would mean frequent and
> unexpected thread creates and joins (and hangs if the thread is not ready to
> be joined). So it's dangerous touching external things in a destructor or
> constructor.
>
> It's maybe not a realistic example but there are other similar reasons why
> touching things outside your class should be left till after construction.

You are correct.

A good solution in this case would be to make AsyncTask noncopyable.

Generally speaking I don't mess with external state in constructors and destructors unless its the purpose of the class to do that, and I only do it through well defined interfaces (eg, the class takes an interface to another class which is used to set up and tear down state, and those functions are guarunteed to be called through exceptions)

« Back to merge proposal