You say you block the main thread until you're guaranteed that the functions in the new thread have secured its access to the data. So why not do this:
Code:
HANDLE hEvent;
void foo2(pp<int>& pTest)
{
Sleep(1000); // Simulate doing some work for 1s
int n = *pTest;
n;
}
DWORD WINAPI foo(void* pParameter) // Wrapper function; takes param and passes it to our processing functions
{
// Thread entry. Fetch variable we passed along and pass it on to our processing functions.
ppnew<int>* p = (ppnew<int>*)pParameter;
pp<int> pThreadCopy = *p; // make a copy here
SetEvent(hEvent); // Tell main thread to continue
foo2(*pThreadCopy); // this works!
return 0;
}
BOOL CKantanAnimeApp::InitInstance()
{
//foo_s* ps = new foo_s;
hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
ppnew<int> OurMemory; // The memory we're fighting over
*OurMemory = 100;
//NewThread(&CKantanAnimeApp::foo, this, ps->pint);
::CreateThread(NULL, NULL, &foo, &OurMemory, NULL, NULL); // Begin race condition
WaitForSingleObject(hEvent, INFINITE); // Wait for the new thread to be ready
/* Sleep(1000); // Simulate some work for 1s*/
int n = *OurMemory;
//delete ps;
OurMemory = ppnew<int>; // We require some new memory now
// and have no use for the old memory,
// so allocate some new and discard the old.
Sleep(100000); // Keep sleeping so the rest of the code won't
// execute, because it's not part of this example.
n;
...
}
That fixes the problem closer to the source.
You want to ban a practice that isn't essential because it can more easily expose a separate flaw in the program. That might be ok if the thing you want to ban is useless and/or easily encountered. I don't think either is the case here.
It's not easily encountered, because the only way the issue will come up is if you have multiple threads and one thread fails to make a copy of the object before passing it to the function or fails to ensure that the other thread will wait until it's safe to destroy the object. This might be your scenario now, but it's not common because in most cases you'll do the threading code correctly.
Passing by reference is not useless. If you wanted to update the actual value, then you would have to pass by reference. In your case, you are not modifying the value, so foo2 should really use a reference to const. However, a reference to const is also not useless. It is more efficient. Presumably since you are in a multithreaded environment you have synchronization around your reference counting in the smart pointer. (You don't want to check the reference count in one statement and delete the pointer in another or the reference count might change from another thread in between those statements and cause you to delete a referenced pointer.) Passing by reference to const avoids the reference counting and related synchronization that pass by value would require, so passing by reference to const is more efficient. That's why it isn't useless.
I understand your point of view. There are cases where things are banned even if they aren't inherently dangerous because they are useless and can expose other programming flaws (not allowing non-const references to temporaries is an example). I just think this is not one of those cases. You've got such a specific example that would be solved by proper coding practices that it isn't worth it to ban a marginally useful feature.
>> That's only done through pass by value.
I don't understand this. You should be passing by value to the thread. If that's not technically possible, then simulate it.