C++ Question

[mcostalba]

Proper RAII is to wrap the thread in a class (preferably a std::unique_ptr), where the constructor creates and the destructor cleans up. This way, whenever an exception happens after the thread has been created, the destructor is guaranteed to be called.

Yes, but this is another topic form what has been raised here.

Please note that also in the previous code we used pointers to threads objects and so if not explicitly deleted we had a leak.

The fix discussed in this thread was for the call of a virtual function in the c'tor of a base class....and I think it has been fixed.

What you raise now is the concern about using a simple pointer instead of a smart pointer, here the reason is that I need to put them in a std::vector and std::auto_ptr is not compatible, I'd should use std::unique_ptr as you suggested, but this is C++11 stuff and the base version of SF is C++03 compatible, not C++11.

And, of course, I have absolutely no intention to build up my home grown, partial and ad-hoc smart pointer wrapper just to overcome this limitation. I prefer to live with the fact that if for some reason Threads.exit() is not called we have a leak, but if this happens it means the application is crashing anyhow.

[Rein Halbersma]

Proper RAII is to wrap the thread in a class (preferably a std::unique_ptr), where the constructor creates and the destructor cleans up. This way, whenever an exception happens after the thread has been created, the destructor is guaranteed to be called.

What you raise now is the concern about using a simple pointer instead of a smart pointer, here the reason is that I need to put them in a std::vector and std::auto_ptr is not compatible, I'd should use std::unique_ptr as you suggested, but this is C++11 stuff and the base version of SF is C++03 compatible, not C++11.

OK, fine if your original problem has been fixed. I think it could have been avoided altogether by not relying on virtual functions in the first place (simple composition e.g. instead inheritance), but that's another point

Which versions of gcc/clang/icc/msvc do you or any of the other contributors develop with? AFAIK, std::unique_ptr has been available on all those platforms since 2010 (gcc 4.4, visual studio 2010). Would it really breaking anything if you would pass -std=c++11 or -std=c++0x and start using std::unique_ptr?

This pattern of resource cleanup outside of destructors is pervasive in Stockfish. E.g. there are many places where you call mutex.lock(); /* some other code */ mutex.unlock(); Again this is not exception safe. C++11 again provides exception-safe wrappers (std::lock_guard) in the Standard Library. But I guess the transition to the C++11 <thread> library would really break things

[mcostalba]

Which versions of gcc/clang/icc/msvc do you or any of the other contributors develop with? AFAIK, std::unique_ptr has been available on all those platforms since 2010 (gcc 4.4, visual studio 2010). Would it really breaking anything if you would pass -std=c++11 or -std=c++0x and start using std::unique_ptr?

It doesn't matter. std::unique_ptr is C++11 material, not C++03 and because it is not absolutely needed I do without it.

This pattern of resource cleanup outside of destructors is pervasive in Stockfish. E.g. there are many places where you call mutex.lock(); /* some other code */ mutex.unlock(); Again this is not exception safe. C++11 again provides exception-safe wrappers (std::lock_guard) in the Standard Library. But I guess the transition to the C++11 <thread> library would really break things

There is a C++11 branch that I try to keep in sync with master, if you give a look there you will find there are less (but not zero) lock/unlock pairs.