AfxBeginThread

Here is another example of bad code that I have seen recently. The implementation given below uses AfxBeginThread MFC method for creating a worker thread.

UINT ThreadProc(LPARAM lParam) {     CString* pStr = (CString*)lParam;     // ...        // Some logic/algorithm that consumes “pStr”     // ...         return 0; } // ... class ThreadTestClass { private:     CString m_string; public:     // ...     // Usual constructor/destructor and other class members!     // ...     void ThreadTestMethod()     {         // ...            // Some logic/algorithm         // ...         AfxBeginThread((AFX_THREADPROC)ThreadProc, &m_string);     } }; // ... void RunThreadTest() {     ThreadTestClass myObject;     // ...     // Some logic     // ...     myObject.ThreadTestMethod();     return; }

This implementation has a severe flaw that causes application to crash randomly (not always). If you take a closer look to RunThreadTest method, you can easily make out that myObject instance is created on stack and so get destroyed the moment control flows out of RunThreadTest method. You may wonder, so what! It works that way. Take a further close look, ThreadTestMethod internally calls MFC API for creating the worker thread and passes pointer to member variable m_string as a parameter to the API. Note that when myObject is destroyed m_string is also destroyed as the part of the destruction of the myObject. Hence, there is a chance that by the time ThreadProc tries to dereference the parameter pStr, m_string would have already destructed. On such incident, the application would simply crash.

The solution to this problem is very simple i.e. maintain the lifetime of the value of pStr properly. In other words, pStr must point to the valid address until its usage is over. One solution is to send a copy of CString parameter created on the heap and let ThreadProc destroy the parameter at the end.

UINT ThreadProc(LPARAM lParam) {     CString* pStr = (CString*)lParam;     // ...        // Some logic/algorithm that consumes “pStr”     // ...         delete pStr;     return 0; } // ... class ThreadTestClass { private:     CString m_string; public:     // ...     // Usual constructor/destructor and other class members!     // ...     void ThreadTestMethod()     {         // ...            // Some logic/algorithm         // ...         AfxBeginThread((AFX_THREADPROC)ThreadProc,                         new CString(m_string));     } }; // ... void RunThreadTest() {     ThreadTestClass myObject;     // ...     // Some logic     // ...     myObject.ThreadTestMethod();     return; }