Bug in Boost.Thread?

I noticed a potential hazard in exceptions.cpp of the thread library. The method: const char* thread_exception::message() const returns the c_str of a string with local scope. This is a problem at least in VC 7 .NET 2003. The error can be exposed by the following code. try { throw boost::lock_error(1); } catch(boost::lock_error& e) { cerr << e.message(); } Thanks, John

John Eddy wrote:
I noticed a potential hazard in exceptions.cpp of the thread library. The method:
const char* thread_exception::message() const
returns the c_str of a string with local scope. This is a problem at least in VC 7 .NET 2003. The error can be exposed by the following code. ...
This is true. This "bug" already has been reported (somewhere Dezember 2004): Ulrich Eckhardt wrote:
The code in question is this:
// boost/thread/exceptions.hpp class thread_exception { ... const char* message() const; }; // libs/thread/src/exceptions.cpp std::string system_message(int sys_err_code){..} const char* thread_exception::message() const { if (m_sys_err != 0) return system_message(m_sys_err).c_str(); return what(); }
If message() is called and m_sys_err is set, it will return a pointer to storage of a temporary, which is bad, as we all know.
I did some research on that, and I think this whole method can be removed completely because: - it is not decumented - it is not used - it never worked (probably, at least not with 1.32)
Further, one could then remove <string> from the includes in the header and some more includes in the sourcefile.
An alternative approach would be to change the returntype to std::string, of course, or (and I think that's an interesting alternative!) derive all those classes from std::runtime_error, which can hold a std::string natively.
Since the function does not yet belong to the documented interface, please don't use it. Use what() instead. Obviously to make message() work, it would need to have an additional string member, or as the above poster points out return a string by value. The second method (deriving from runtime_error) won't solve the problem in my opinion, since 1) We need two strings: one that is returned by what() and another returned by message() 2) We cannot access the string at the time message() is beeing called. We would need to do this at construction time, whether it will be used later or not. I would even prefer not to make message a member function at all. The exception simply should return the system error code, and a free message function beeing used to convert it to a meaningful string representation. Besides this, the whole issue is a mere theoretical one, since the various exceptions are not yet thrown with system codes attached. But perhaps I am wrong in this respect. How did you discover the "problem"? Roland

Roland Schwarz wrote:
John Eddy wrote:
I noticed a potential hazard in exceptions.cpp of the thread library. The method:
const char* thread_exception::message() const
returns the c_str of a string with local scope. This is a problem at least in VC 7 .NET 2003. The error can be exposed by the following code. ...
This is true. This "bug" already has been reported (somewhere Dezember 2004):
Ulrich Eckhardt wrote:
The code in question is this:
// boost/thread/exceptions.hpp class thread_exception { ... const char* message() const; }; // libs/thread/src/exceptions.cpp std::string system_message(int sys_err_code){..} const char* thread_exception::message() const { if (m_sys_err != 0) return system_message(m_sys_err).c_str(); return what(); }
If message() is called and m_sys_err is set, it will return a pointer to storage of a temporary, which is bad, as we all know. I did some research on that, and I think this whole method can be removed completely because: - it is not decumented - it is not used - it never worked (probably, at least not with 1.32)
Further, one could then remove <string> from the includes in the header and some more includes in the sourcefile.
An alternative approach would be to change the returntype to std::string, of course, or (and I think that's an interesting alternative!) derive all those classes from std::runtime_error, which can hold a std::string natively.
Since the function does not yet belong to the documented interface, please don't use it. Use what() instead.
Obviously to make message() work, it would need to have an additional string member, or as the above poster points out return a string by value. The second method (deriving from runtime_error) won't solve the problem in my opinion, since 1) We need two strings: one that is returned by what() and another returned by message() 2) We cannot access the string at the time message() is beeing called. We would need to do this at construction time, whether it will be used later or not.
I would even prefer not to make message a member function at all. The exception simply should return the system error code, and a free message function beeing used to convert it to a meaningful string representation.
Besides this, the whole issue is a mere theoretical one, since the various exceptions are not yet thrown with system codes attached. But perhaps I am wrong in this respect. How did you discover the "problem"?
I was evaluating the risk of an exception being thrown in my application. So I was looking through the code. It did not happen to me. I did see that the thread exceptions never seem to be used in that way by the thread library anyway. Perhaps a solution (ugly as it may be) is to define a PP sequence of system errors for each platform/compiler (are they well standardized?) and PP enumerate private message_# methods (or perhaps non-member message# functions in exceptions.cpp so the macros needn't be in the header), which are switched on in the public message method (switch cases also PP enumerated). Each message_# method would return an appropriate text. Thanks John
Roland
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
participants (2)
-
John Eddy
-
Roland Schwarz