|
I have an MFC dialog application. It monitors certain types of windows and collects information on them. I start a new thread for every window based on it's title. I use a timer to find windows that meet the criteria. I created a struct named ThreadInfo and I pass it to the thread. At the end I either close the monitored window(s) or close my dialogs.
This causes several memory leaks.
Here is the code in the timer if it finds a window that meets the criteria:
ThreadInfo *pInfo = new ThreadInfo;
pInfo->hMainWnd = dlg->GetSafeHwnd();
pInfo->hMonitoredWnd = hwnd;
pInfo->x = y;
CWinThread *pThread = AfxBeginThread(ThreadFunction, (PVOID) pInfo, THREAD_PRIORITY_NORMAL, 0, 0);
Here is the code for ThreadFunction:
UINT ThreadFunction(LPVOID pParam)
{
ThreadInfo *pInfo = reinterpret_cast<threadinfo> (pParam);
while (IsWindow(pInfo->hMonitoredWindow))
{
do.Something;
Sleep(100);
}
return 0;
}</threadinfo>
I tried the following scenarions to eliminate or at least identify the memory leaks. The resulting leaks are below them:
A:
- I create pInfo and close the dialog first:
thrcore.cpp 68B client block
mydlg.cpp 748B normal block
strcore.cpp 46B normal block
B:
- I create pInfo and close the monitored window first:
mydlg.cpp 748B nromal block
strcore.cpp 46B normal block
C:
- I create pInfo, but don't start the thread:
mydlg.cpp 748B normal block
strcore.cpp 46B normal block
D:
- I don't create pInfo and pass NULL to the thread:
no leaks!
E:
- I delete pInfo in ThreadFunction and close monitored window first:
no cpp file, 160B normal block
F:
- I delete pInfo in ThreadFunction and close dialog first:
no cpp file, 160B normal block
thrcore.cpp 68B client block
mydlg.cpp 748B normal block
strcore.cpp 46B normal block
So I think the 748B and 46B blocks are for pInfo, while the 68B and 160B block are for pThread. But how and where should I delete them?
I think I should delete them in the dialog, but how do I send a message from the thread when it ends? And how I delete the handles if I close the dialog first?
|
|
|
|
|
1. Why not take a local copy of *pInfo early in the thread function, so you can delete pInfo early:
UINT ThreadFunction(LPVOID pParam)
{
ThreadInfo info(reinterpret_cast<ThreadInfo*>(pParam));
delete reinterpret_cast<ThreadInfo*>(pParam);
while (IsWindow(info.hMonitoredWindow))
{
do.Something;
Sleep(100);
}
return 0;
}
2. Do you keep track of all the threads you've created? Doesn't look like it. Store them in a vector or something and delete them in your dialog's WM_CLOSE handler.
3. If the monitored windows aren't closed, does the thread terminate? You probably ought to have some way of telling the threads to exit even if their monitored window still exists.
Basically, you need to think about the likely lifetimes of objects and work out where you need to be deleting objects.
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Thanks!
1. This didn't compile, error was:
error C2664: 'ThreadInfo::ThreadInfo(const ThreadInfo &)' : cannot convert parameter 1 from 'ThreadInfo *' to 'const ThreadInfo &'
but I used the following:
ThreadInfo *pInfo = reinterpret_cast<threadinfo> (pParam);
ThreadInfo info = *pInfo;
delete pInfo;</threadinfo>
And it seems to work. The 748B and 160B blocks are eliminated.
2. I had a threads vector earlier, but I didn't use it. Now I use it as you recommended, but I use ON_WM_DESTROY (I switched off the close button):
void CFredoDlg::OnDestroy()
{
for(vector<handle>::size_type i = 0; i < threads.size(); i++)
{
threads.erase(threads.begin() + i);
}
}</handle>
But the 68B and 46B blocks are still there. Now I will try to find a way to terminate the threads. If you have a solution for that, don't keep it!
|
|
|
|
|
keret wrote: But the 68B and 46B blocks are still there
Implies you have a thread object and a string that you haven't deleted.
keret wrote: Now I will try to find a way to terminate the threads. If you have a solution for that, don't keep it!
You could use a global boolean-ish variable. You would set it using InterlockedExchange[^] and read as you would any other variable.
Set it to zero before any threads are started and to one when you want to exit. Use this code to set it (let's presume it's called letsExitNow )
InterlockedExchange(&letsExitNow, 1);
Now, in each thread, you would change your with loop to somethig like this:
while (!letsExitNow && IsWindow(pInfo->hMonitoredWindow))
In the ON_WM_DESTROY handler, wait for each thread handle to be signalled (signalled means the thread has exited) - you can use WaitForSingleObject[^] or WaitForMultipleObjects[^] to do that.
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
My TreadFunction is in a separate file (MyThread.cpp).
How can I reference letsExitNow from there? Or should I put the code in MyDlg.cpp?
|
|
|
|
|
Add a definition for letsExitNow in one file:
LONG letsExitNow;
and an external declaration in the other:
extern LONG letsExitNow;
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Sorry, I have never used extern before.
I put this in MyDlg.h:
public:
LONG letsExitNow
I put this in CMyDlg::OnInitDialog:
InterlockedExchange(&letsExitNow, 0);
And I put this in ThreadFunction:
extern LONG letsExitNow;
But I got an "unresolved external symbol "long letsExitNow"" error. Did I something wrong?
|
|
|
|
|
Minor mis-understanding
Put the LONG letsExitNow in MyDlg.cpp - NOT in a method, just in the file.
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Thank You, I really appreciate your help. This was a complicated problem (for me now at least). The 160B object was another struct I didn't include in the code above.
|
|
|
|
|
Hi all,
I am developing a VC++ console application in which I have encountered a very strange problem. I have a class class A in which there are member objects of other different classes say class B and C . In Class A there is a member function search(int condition) . This search() function depending upon argument returns a pointer to the member objects with the class. Code for this is:
class B{
};
class C{
};
class A{
public:
B obj1;
C obj2;
someThing* search(int condition)
{
if(condition==1)
return &obj1;
if(condition==2)
return &obj2;
if(condition==3)
return this;
}
};
Now the problem is that I am not able to specify any return type since it depends upon the condition which is known at runtime. I tried to introduce template as:
template -typename type-
type* search(int condition)
{
}
But, doing so I cannot call the function from main() since, while calling I have to specify template parameter which again is dependent on condition at run-time. I am empty headed and clueless rite now about solving this weird situation.. Please suggest any problem solving approach. Also to mention that class structure is not to be changed.... HELP!!
|
|
|
|
|
NeoAks007 wrote: Now the problem is that I am not able to specify any return type since it depends upon the condition which is known at runtime. I tried to introduce template as:
You can't use templates here since your type is determined at run time. Create a abstract class and derive class B and C from it. You can return pointer to this abstract class instantiated with proper derived one.
|
|
|
|
|
Hmmmm.. That seems to be a nice and elegant solution... Let me try it before I thank you (since actual class hierarchies are quite complex so implementing this solution might introduce new errors) !!! ..
|
|
|
|
|
Good luck then.
|
|
|
|
|
N a v a n e e t h wrote: You can return pointer to this abstract class instantiated with proper derived one.
Can you demonstrate how to do it?
|
|
|
|
|
class DummyBase {};
class B : public DummyBase{
};
class C : public DummyBase{
};
class A{
public:
B obj1;
C obj2;
DummyBase* search(int condition)
{
if(condition==1)
return &obj1;
if(condition==2)
return &obj2;
if(condition==3)
return this;
}
};
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Here is a trivial example.
class Car{
public:
virtual string SayName() = 0;
};
class Mercedez : public Car{
public:
string SayName(){
return "Mercedez";
}
};
class Ferrari : public Car{
public:
string SayName(){
return "Ferrari";
}
};
Car* CreateACar(string carType){
if(carType == "Mercedez")
return new Mercedez;
else if(carType == "Ferrari")
return new Ferrari;
} Use it like
Car* car = CreateACar("Mercedez");
std::cout << car->SayName(); Read about factory design pattern.
|
|
|
|
|
I had implemented DummyBase this way only. But there was problem accessing functions of class B & C through main() . After figuring out why, I have another serious problem now. Actual Implementations of
class B and C are:<br />
<pre><code><br />
class DummyBase {};<br />
<br />
template <'class abc'><br />
class Base{<br />
public:<br />
abc func1();<br />
};<br />
<br />
class B : public Base`<datatype1`>, DummyBase{<br />
};<br />
class C : public Base`<datatype2`>, DummyBase{<br />
};<br />
<br />
class A: public Base`<datatype3`>, DummyBase{<br />
public:<br />
B obj1;<br />
C obj2;<br />
DummyBase* search(int condition)
{<br />
if(condition==1)<br />
return &obj1;<br />
if(condition==2)<br />
return &obj2;<br />
if(condition==3)<br />
return this;<br />
}<br />
};<br />
<br />
void main()<br />
{<br />
DummyBase *ptr;<br />
A objA;<br />
int n;<br />
cin>>n;<br />
ptr = objA.search(n);<br />
ptr->func1();
}<br />
Now the problem is that func1() cannot be included in DummyBase since return type of func1() is a template parameter. How to solve this.....
|
|
|
|
|
Plz do reply posting solution to my problem (other than restructuring class Hierarchies). It quite urgent!!
|
|
|
|
|
That's a tricky one. I'd probably try it like this:
Add virtual destructors to DummyBase, B and C, to ensure a v-table. Then you can use dynamic_cast to work out which typ has been returned:
class DummyBase { virtual ~DummyBase() {} };
template <'class abc'>
class Base{
public:
abc func1();
};
class B : public Base`<datatype1`>, DummyBase{
virtual ~B() {}
};
class C : public Base`<datatype2`>, DummyBase{
virtual ~C() {}
};
class A: public Base`<datatype3`>, DummyBase{
public:
B obj1;
C obj2;
DummyBase* search(int condition)
{
if(condition==1)
return &obj1;
if(condition==2)
return &obj2;
if(condition==3)
return this;
}
};
void main()
{
DummyBase *ptr;
A objA;
int n;
cin>>n;
ptr = objA.search(n);
if (B* bPtr = dynamic_cast<B*>(ptr))
{ bPtr->func1(); }
if (C* bPtr = dynamic_cast<C*>(ptr))
{ bPtr->func1(); }
}
Yeah, it sucks because you're taking so many decisions. There are probably better ways, but I've not really put sufficient thought into it to think of a better one.
Possibly something like Boost.Variant or Boost.Any might make things nicer.
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Stuart Dootson wrote: Yeah, it sucks because you're taking so many decisions.
Yes, that's a quite heck of a job. Actually search() function is like a framework function which links 2 separate applications. Now if this approach is to be implemented then I have to do a lot of coding at caller side. Also there are about 40-50 functions(may increase with further development) which would call search . And this approach is quite rigid and for instance if new class M object is introduced in class A a lot work would be required. So a more efficient flexible solution is required.
Stuart Dootson wrote: But I've not really put sufficient thought into it to think of a better one.
If possible could you please spare some time to think of a better one??? I have nearly wasted a day trying to find workarounds for it and now i cannot think more since here in India it's midnight now and i'm feeling sleepy.
Stuart Dootson wrote: Possibly something like Boost.Variant or Boost.Any might make things nicer.
I am not familiar with Boost libraries. So how could they be implemented??
|
|
|
|
|
With Boost, this is the best I can think of (note that I've substituted int, cahr, float for the template parameters):
#include <iostream>
#include <boost/variant.hpp>
template <class abc>
class Base{
public:
abc func1();
};
class B : public Base<int>{
public:
virtual ~B() {}
};
class C : public Base<float>{
public:
virtual ~C() {}
};
class A: public Base<char>{
public:
B obj1;
C obj2;
boost::variant<A*, B*, C*> search(int condition)
{
if(condition==1)
return &obj1;
if(condition==2)
return &obj2;
if(condition==3)
return this;
}
};
int main(int, char**)
{
A objA;
int n;
std::cin>>n;
boost::variant<A*, B*, C*> v = objA.search(n);
if (B* b = boost::get<B*>(v))
{
b->func1();
}
else if (C* c = boost::get<C*>(v))
{
c->func1();
}
return 0;
}
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Thanks for sparing some time.... But isn't this approach similar to previous approach??? Still I have to code a lot at caller side as in last solution... Is there no way to reduce coding at caller side??
|
|
|
|
|
NeoAks007 wrote: Is there no way to reduce coding at caller side??
Not really - you have different return types for the different implementations of func1 - you have to handle those differently. If you want to handle them n the same way, then why not make them the same?
Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p
|
|
|
|
|
Stuart Dootson wrote: If you want to handle them n the same way, then why not make them the same?
It cannot be made same because their functionalities are different in different classes... I understand that this is holiday but still if anyone reading this thread comes up with an idea, do post a solution please... I need it seriously..... I am still listening.....
modified on Sunday, March 15, 2009 1:36 PM
|
|
|
|
|
I think you're making this way too complicated. If you change the return type, the code handling that will have to change as well.
Anyone who thinks he has a better idea of what's good for people than people do is a swine.
- P.J. O'Rourke
|
|
|
|