|
valerie99 wrote: Is this the proper delete?
Yes, except you don't need to check if Groups is not null, before deleting. If Groups is null then delete simply does nothing.
My question is: Are you realy trying to access Groups after deleting them.
DiscountData->GroupID = Groups[i].GroupID;
INTP
Every thing is relative...
|
|
|
|
|
thanks for your reply.
if I step into the program, it's actually working by this float:
if ( Groups )
{
delete[] Groups;
Groups = NULL;
}
NumberofGroups = this->GetContributoryAmounts (mainPos);
Groups = new GROUPSTRUCT[NumberofGroups];
for ( i = 0; i < NumberofGroups; i++ )
{
Discount->GetGroup( Groups[i].GroupID );
Groups[i].TotalContributory = 0.0;
}
for ( i = 0; i < NumberofGroups; i++ )
{
DiscountData->GroupID = Groups[i].GroupID;
DiscountData->TotalContributory = Groups[i].TotalContributory;
|
|
|
|
|
valerie99 wrote: the app detected memory leak at this point:
Clicking on the line in the Debug window should take you to the spot where the new operator was used. That is your memory leak.
valerie99 wrote: Is this the proper delete?
No, because you are accessing Groups[i] after Groups has already been deleted.
valerie99 wrote: DiscountData->GroupID = Groups[i].GroupID;
I would have at least expected an access violation from this.
"Take only what you need and leave the land as you found it." - Native American Proverb
|
|
|
|
|
thanks. here is the memory leak report. when I clicked the first line it brings to "Groups = new GROUPSTRUCT[NumberofGroups];", I think it's becaue of "you are accessing Groups[i] after Groups has already been deleted", strangly no access violation.
:\dev\c++\billing\voldisc\voldiscdlg.cpp(1702) : {839} normal block at 0x00FE58F0, 16 bytes long.
Data: < > 01 00 00 00 CD CD CD CD 00 00 00 00 00 00 00 00
{465} normal block at 0x00FE5230, 1309 bytes long.
Data: 48 65 61 64 65 72 20 4C 65 6E 67 74 68 3A 20 30
{460} normal block at 0x00FC28B8, 60 bytes long.
Data: < a 0R > 04 EA 61 00 30 52 FE 00 1D 05 00 00 1D 05 00 00
{210} normal block at 0x00FBDB80, 14844 bytes long.
Data: <5 a > 35 00 00 00 90 D2 61 00 98 81 13 00 00 00 00 00
{209} normal block at 0x00FBDB28, 20 bytes long.
Data: <billdata\vtax02.> 42 49 4C 4C 44 41 54 41 5C 56 54 41 58 30 32 2E
{208} normal block at 0x00FBDAC0, 36 bytes long.
Data: < a 5 ( > B8 CC 61 00 35 00 00 00 28 DB FB 00 98 81 13 00
Object dump complete.
So the major leak is {210} normal block at 0x00FBDB80, 14844 bytes long.
Data: <5 a > 35 00 00 00 90 D2 61 00 98 81 13 00 00 00 00 00 , which I couldn't get there by clicking on, so I placed this line _CrtSetBreakAlloc(210); to set a user break point, here is the call stack when the breakpoint taken effect.
_heap_alloc_dbg(unsigned int 14844, int 1, const char * 0x00000000, int 0) line 338
_nh_malloc_dbg(unsigned int 14844, int 0, int 1, const char * 0x00000000, int 0) line 248 + 21 bytes
_malloc_dbg(unsigned int 14844, int 1, const char * 0x00000000, int 0) line 165 + 27 bytes
operator new(unsigned int 14844) line 325 + 15 bytes
CB::Open(const char * 0x00e29f69, int 0) line 915 + 27 bytes
CB::CB(const char * 0x00e29f69, int 0) line 883
CB_DBF::CB_DBF(const char * 0x00e29f69, int 0) line 269 + 45 bytes
OpenDbfHelper(const char * 0x00e29f69) line 93 + 40 bytes
OpenDbf(CB_DBF * * 0x0064acf0 class CB_DBF * VTax, const char * 0x00e29f69, int & 0, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * 0x00fafdcc {0x00000000 ""}) line 46 + 9 bytes
CVoldiscDlg::OpenDBFs() line 838 + 27 bytes
Run(void * 0x0012fd5c) line 2557 + 8 bytes
_AfxThreadEntry(void * 0x0012f6cc) line 112 + 13 bytes
_threadstartex(void * 0x00e24b90) line 212 + 13 bytes
KERNEL32! 7c57b388()
could u give me some help about how to nerrow down where the leak starts? thank you !
|
|
|
|
|
valerie99 wrote: strangly no access violation
Some times the exception is generated after it occurs, when you are in an etirely different place in your code. Thats why they are so hard to track down, where the exception is generated and where it actualy occured may be to totaly different places.
If you fix what you can see for a fact is wrong, then the problem may go away.
INTP
Every thing is relative...
|
|
|
|
|
because the "NumberofGroups = this->GetContributoryAmounts (mainPos); " will call the "Groups = new GROUPSTRUCT[NumberofGroups];
", so it's actually not trying to access right after deleting, here is the flow:
if ( Groups )
{
delete[] Groups;
Groups = NULL;
}
NumberofGroups = this->GetContributoryAmounts (mainPos);
Groups = new GROUPSTRUCT[NumberofGroups];
for ( i = 0; i < NumberofGroups; i++ )
{
Discount->GetGroup( Groups[i].GroupID );
Groups[i].TotalContributory = 0.0;
}
for ( i = 0; i < NumberofGroups; i++ )
{
DiscountData->GroupID = Groups[i].GroupID;
DiscountData->TotalContributory = Groups[i].TotalContributory;
|
|
|
|
|
valerie99 wrote: NumberofGroups = this->GetContributoryAmounts (mainPos);
Since you indicate that this will eventually call the new operator, where is the corresponding delete?
"Take only what you need and leave the land as you found it." - Native American Proverb
|
|
|
|
|
According to what you said:
valerie99 wrote: because the "NumberofGroups = this->GetContributoryAmounts (mainPos); " will call the "Groups = new GROUPSTRUCT[NumberofGroups];
so, if this is true, when you do this:
valerie99 wrote: NumberofGroups = this->GetContributoryAmounts (mainPos);
Groups = new GROUPSTRUCT[NumberofGroups];
the first line allocates memory with new and assigns the pointer to Groups, but then you also do your own new and assign it to Groups. Therefore the previous assignment from NumberofGroups = this->GetContributoryAmounts (mainPos); is lost, and a memory leak.
Hope that helps.
Karl - WK5M
PP-ASEL-IA (N43CS)
<kmedcalf@ev1.net>
PGP Key: 0xDB02E193
PGP Key Fingerprint: 8F06 5A2E 2735 892B 821C 871A 0411 94EA DB02 E193
|
|
|
|
|
What does this code snippet produce for you. Some of the values I just guessed since I did not know how you were using them.
class GROUPSTRUCT
{
public:
int GroupID;
double TotalContributory;
};
void main( void )
{
int NumberofGroups = 15;
GROUPSTRUCT *Groups = new GROUPSTRUCT[NumberofGroups];
if (Groups != NULL)
{
for (int i = 0; i < NumberofGroups; i++)
{
Groups[i].TotalContributory = 0.0;
}
delete [] Groups;
Groups = NULL;
}
}Any leaks?
"Take only what you need and leave the land as you found it." - Native American Proverb
|
|
|
|
|
Even I set a thread to realtime_Priority_Class, and PRIORITY_REALTIME. it still cann't gurantee to run at a every 50ms.
My code is like this
ThreadProc()
{
while(1)
{
doSomethingCritical();
Sleep(50);
}
}
|
|
|
|
|
Before digging deep into the hows and whys, read this[^] regarding windows and timing.
Afterwards, have a look at multimedia timers (timeBeginPeriod, timeSetEvent and so on).
The use of multimedia timers will give you the best performance considering windows not being a real-time OS.
BTW, boosting both the thread's and the process' priority to "realtime" will prevent even task manager from running as expected.
Hope this helps
--
Roger
It's supposed to be hard, otherwise anybody could do it!
|
|
|
|
|
First, the article referred to by Roger is out of date and contains some errors [this was edited to make it more clear that I was talking about the article Roger linked to and that I am not saying the article is full of errors. It is not.] Most importantly, a thread does not have to run for it's entire quanta (or time slice) and in reality, most do surrender most of it. Second, W2K and XP use a different resolution on timers [Edit: this is misleading. I meant to say that W2K and XP are different than NT, not necessarily from each other. W2K, for example, allows much more flexibility over the quantum table: One significant change to W2K
http://www.sysinternals.com/Information/Windows2000Quantums.html[^]]
The article is [very] right that that Windows is NOT a real time OS. It cannot guarantee that a thread will execute at a set time unless you add specialized hardware and drivers. However, it generally responds within 1ms of your ideal--even WM_TIMER is pretty accurate if you're talking 50ms. [I stand by this; note the phrase "in general."]
Take a look at SetWaitableTimer for one option.
Another is to use WaitForSingleObject and use a timeout that you calculate using GetTickCount() [Edit: QueryPerformanceTimer() is a better choice] with each loop based on how much time the operation of the loop took. I've used this for applications where I needed behavior like SetWaitableTimer() but was severely limited on the number of handles I could use.
Anyone who thinks he has a better idea of what's good for people than people do is a swine.
- P.J. O'Rourke
-- modified at 12:20 Wednesday 26th October, 2005
|
|
|
|
|
Joe Woodbury wrote: the article referred to by Roger is out of date and makes several errors
If this is so, please provide some links to MSDN or similar. Otherwise this is just an opinion which I disagree with and my opinion is based on my own experience on various windows OS versions.
Joe Woodbury wrote: a thread does not have to run for it's entire quanta
True. But this is not done by the OS scheduler. It requires the thread to call either ::Sleep(0) or one of the ::WaitForXXXObject functions.
For the thread to relinquish its time slice by calling ::Sleep(0) requires that there is another thread of equal or higher priority that is ready to run, otherwise the thread is resumed.
This is not a matter of opinion, you can read about it in MSDN[^].
Joe Woodbury wrote: W2K and XP use a different resolution on timers
What timers? Is this another opionion or do you have links?
As an example I've tried the same code running on WinNT4 and WinXP with a better result regarding response time on an older P2 machine running WinNT4 than a fairly new P4 running WinXP using waitable timers.
Regardless of how I designed my executable this would not be the case if what you claim was the fact.
Joe Woodbury wrote: it generally responds within 1ms of your ideal
This I have never experienced on any version of windows without the use of multimedia timers.
How have you measured this?
Joe Woodbury wrote: even WM_TIMER is pretty accurate if you're talking 50ms
Do you really think he meant ±50ms when he has boosted both the priority of the process and the thread to "realtime"?
I get the impression that he want a timer event to occur every 50ms with the highest precision available.
To even mention WM_TIMER in such case is absurd since WM_TIMER is a low priority message and will only be generated if there are no other messages in the thread's message queue. This means that moving the mouse around above your window could delay the WM_TIMER message to be generated/handled.
Again this can be read in MSDN[^].
To use ::WaitForSingleObject to wait for a waitable timer could be a solution.
When the timer event is signalled, the waiting threads priority is boosted by 1 to make the scheduler run the waiting thread ASAP.
A better alternativ than ::GetTickCount() regarding measuring time is the use of ::QueryPerformanceCounter().
Don't make the mistake assuming that something has the same precision as the least significant bit of the value, i.e. ::Sleep(1) does not necessarily generate a delay of one millisecond.
--
Roger
It's supposed to be hard, otherwise anybody could do it!
|
|
|
|
|
(Failing to learn lessons from the past, I wrote my original reply at 3:00 in the morning. I have taken the liberty to totally edit this. I belately realized I did not make several things clear including that I was referring to the article Roger referenced, not the comment Roger made.)
First, I did not mean to imply the article Roger referenced is horribly wrong. It is out of date; it was written five years ago and ends with NT 4 Alpha information which I think may be incorrect--I believe NT 4 still uses 10 ms resolution.) Since then Windows XP, Windows Server 2003 and several iterations of Windows CE have been released. Windows CE 5.0 is most definitely an RTOS. (W2K has much more flexibilty in controlling quantum lengths as outlined in this article: http://www.sysinternals.com/Information/Windows2000Quantums.html[^].)
The article does contain some innacuracies. For example, thread quanta are determined by clock ticks, not by a set time. The HAL determines clock ticks. The author implies this, but doesn't state it clearly.
Second, I did not make it clear in my original posting that I had made the tacit assumption we were talking about the predictability of a 50ms timer as stated in the original question.
Third, my response was dealing with typical behavior, which in Windows is all you can really do. In practice, Windows threads rarely use their entire quanta. This means that various solutions are possible depending on how much "slop" you want in your system.
In connection with this, the Windows message queue is rarely full of high priority messages (I have a vague recollection of a undocumented caveat to the WM_TIMER documentation, though I may be mistaken.)
Fourth, Roger asked if I "really think he meant ±50ms when he has boosted both the priority of the process and the thread to "realtime"?"
I did not say anything about ±50ms; again I assumed, perhaps erroneously, that the original poster wanted some code to execute every 50ms. Since Windows is not an RTOS, a Windows programmer must accept that in the best case, you must accept a tolerance of at least +1ms. Also, THREAD_PRIORITY_TIME_CRITICAL threads in REALTIME_PRIORITY_CLASS processes are a really bad idea, and rarely needed, unless you know exactly what you are doing. I attempted to find information on thread inversion behavior, if any, with this situation and could not.
A good article on this: http://www.microsoft.com/mspress/books/sampchap/4354c.asp[^]
Once you accept that there will be some inaccuracy, the only question is how much is tolerable. (In my own experience working on both hard and soft RTOS systems, this unpredictability is very often the bigger problem.) WIth this in mind, a much wider range of solutions are possible. (QueryPerformanceCounter() is a better solution than GetTickCount(), though the latter will, again, work depending on your criteria.)
It is my experience is that developers new to multi-threading, myself included, often start playing with thread priority thinking that will solve problems that are ultimately problems with design. I think that is the case here; he is forgetting the time used by the task and is thus simply adding a 50ms delay between task execution, not doing something roughly every 50ms.
Finally, I have measured timings by comparing internal x86 counters against the timing of threads as well as using QueryPerformanceCounter().
-- modified at 12:20 Wednesday 26th October, 2005
|
|
|
|
|
Hi everyone,
I need some help with a design problem that I have. I am not exactly a software developer by profession, so I am hoping to get some suggestion from the experts here.
Well, basically I have 2 types of tracking systems (optical and sonar). So, basically I have an interface object and they both should inherit from it.
However, my problem is that they both take different parameters for initialization. So, if I have a method calloed Initialize, I cannot share it between then as they take different parameters for initializazion.
I was wondering what kind of design patterns you guys would recommend for developing objects that are of course same from a functional point of you, but differ internally quite substantially.
Thanks and cheers,
Keith
|
|
|
|
|
Hi,
do users set the parameters needed for initialization at runtime ?
We can do no great things, only small things with great love. - Mother Theresa
|
|
|
|
|
Normaly initialization only needs to be done when the object if first created, at which point in time you already know what type it is. Given that, use an abstract base class that only provides the interfaces they have in common and provide each derived class with an initialization function specific for that class. Do not make the initialization function part of the base class, because it only provides the interface that you need to access after initialization.
If for some reason you need to reinitialize the object thru a pointer to the base class (unusual). You'll need to know what all the derive classes are so you can use dynamic_cast<>() to downcast the base class pointer to one of the derived types. At which time you will know what type it is and what paramenters are required.
INTP
Every thing is relative...
|
|
|
|
|
|
This is quite common; the derived objects have their own constructors. They each can call the base constructor (or Init() function) with those parameters that are in common.
If the object has to be created early and then fully initialized later, you can create two virtual functions in the base object, one for each type. Yes, it's ugly, but I've done worse.
Anyone who thinks he has a better idea of what's good for people than people do is a swine.
- P.J. O'Rourke
|
|
|
|
|
Create parameter classes derived from a common base.
something like this
class TrackingParams
{
...
}
class OpticalTrackingParams : public TrackingParams
{
...
}
class SonarTrackingParams : public TrackingParams
{
...
}
class TrackingInterface
{
public:
virtual void Init(TrackingParams &);
};
class OpticalTracking : public TrackingInterface
{
void Init(TrackingParams & params)
{
OpticalTrackingParams & p = dynamic_cast<opticaltrackingparams &="">(params);
}
}
|
|
|
|
|
Im having a little trouble using free after a malloc.
Here is the malloc part:
GUIQueue* newMsg = (GUIQueue*)malloc( sizeof(GUIQueue) );
newMsg->data = (char*)malloc( sizeof(msg.u.reqvid) );
...
GUIQ.push( newMsg );
What i do now is pull the info from the queue (this is a MT'ed app.) and get the info and free the pointer.
GUIQueue* newMsg;
newMsg = (GUIQueue*)GUIQ.front();
GUIQ.pop(); // I know this looks weird, but pop doesnt return anything, so...
...
if( newMsg->data != NULL )
{
free(newMsg->data); // This first free causes the runtime error.
newMsg->data = NULL;
}
if( newMsg != NULL )
{
free(newMsg);
newMsg = NULL;
}
The error message that pops up is:
Unhandled exception at 0x7c901230 in LiteCycles.exe: User breakpoint.
I've cleared all my breakpoints, so i dont think it can be that, the stack looks like its deep in free and the last legiable method is _CrtIsValidHeapPointer.
Any ideas on whats causing this error? Any input is greatly appricated, thanks.
-Jader89
"There are 10 types of people, those who understand binary, and those who don't."
- Somebody, not me.
|
|
|
|
|
You are using either a class or a template that is called "GUIQueue".
Never mix C++ objects with malloc/free. Use new/delete instead.
Why? Because the object's constructor and destructor won't get called when you are using malloc and free.
What kind of errors you get depend on the C++ object you are using.
--
Roger
It's supposed to be hard, otherwise anybody could do it!
|
|
|
|
|
I tried switching to delete and new, but down the line of delete it still runs into the same method that free does, and cause the same error. Upon review of the last method, in the method header info, it says that it checks to see if the pointer is in the local heap, which i dont think it is in the local heap, cause its a different thread thats creating it, so this could be the main problem.
"There are 10 types of people, those who understand binary, and those who don't."
- Somebody, not me.
|
|
|
|
|
I'm not really familiar with multithreaded application, but I think you will need to block the
access to your data when you free it; it looks like one of your thread will try to access the data while your are free-ing it.
have a look at mutex, critical section, and all the other related mechanism for locking access to your data to only one thread.
Maximilien Lincourt
Your Head A Splode - Strong Bad
|
|
|
|
|
I think I see what your saying, but the only thing that accesses the data after its created, is the same thread that free()'s it. So its not something else trying to access the data. Is that what you were getting at?
"There are 10 types of people, those who understand binary, and those who don't."
- Somebody, not me.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
