Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
HUGE MEMORY LEAK pinpointed!
rickg22:
--- Quote from: dmoore on June 23, 2007, 03:13:25 pm ---PS: the cc code is spaghetti with (IMHO) an unnecessarily complex threading model.
--- End quote ---
The complex threading model was necessary, trust us. Parsing 1,000 files in the main thread is simply out of the question.
But I agree 100% with you on the spaghetti. I'm researching and testing a new model for Code Completion to implement after v1.0 is released. It's a longterm project.
rickg22:
--- Quote from: dmoore on June 23, 2007, 03:13:25 pm ---hacky patch here: http://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=2075&group_id=5358
--- End quote ---
The patch has an error.
--- Code: ---opts.loader=Manager::Get()->GetFileManager()->Load(bufferOrFilename, false);
--- End code ---
Should be:
--- Code: ---if(!opts.loader)
{
opts.loader=Manager::Get()->GetFileManager()->Load(bufferOrFilename, false);
}
--- End code ---
Otherwise you'd be overwriting the loader in case the function gets called somewhere else. Please fix and resend the patch :)
thomas:
--- Quote from: dmoore on June 23, 2007, 03:13:25 pm ---(filemanager seems to require callers to release loaders, but only after they have finished loading??)
--- End quote ---
Yes, and yes. However, in the case of deleting loaders before they have finished loading, the memory leak is the least of your problems, since you're referencing unallocated memory all the way.
This is a well-known behaviour of FileManager that was discussed when it was first created.
I haven't looked into the code completion code, but if it does something like open many files and delete the loaders right away, then this sure explains why memory is leaked.
rickg22:
Thomas, what do you think about the patch? Is it OK with you?
dmoore:
--- Quote from: rickg22 on June 23, 2007, 08:11:55 pm ---The patch has an error.
--- Code: ---opts.loader=Manager::Get()->GetFileManager()->Load(bufferOrFilename, false);
--- End code ---
Should be:
--- Code: ---if(!opts.loader)
{
opts.loader=Manager::Get()->GetFileManager()->Load(bufferOrFilename, false);
}
--- End code ---
Otherwise you'd be overwriting the loader in case the function gets called somewhere else. Please fix and resend the patch :)
--- End quote ---
in my patch, I NULLed out the loader member of opts in all instances where Parse is called and needs a loader, so (!opts.loader) should always be true. More importantly, in the present state of the plugin, the loader should never be called (using Load) outside of Parse or there will be memory leaks, so loader must always be NULL unless you come up with a more elaborate patch.
--- Quote from: thomas on June 24, 2007, 01:42:09 am ---
--- Quote from: dmoore on June 23, 2007, 03:13:25 pm ---(filemanager seems to require callers to release loaders, but only after they have finished loading??)
--- End quote ---
Yes, and yes. However, in the case of deleting loaders before they have finished loading, the memory leak is the least of your problems, since you're referencing unallocated memory all the way.
This is a well-known behaviour of FileManager that was discussed when it was first created.
I haven't looked into the code completion code, but if it does something like open many files and delete the loaders right away, then this sure explains why memory is leaked.
--- End quote ---
No, CC never deletes the loaders that aren't complete (this causes seg faults), it just fails to delete some loaders and loaders don't delete themselves because they have to store the file data for the caller to use. There are lots of different ways to address this issue - my approach was just the simplest that came to mind.
--- Quote from: rickg22 on June 23, 2007, 08:05:29 pm ---The complex threading model was necessary, trust us. Parsing 1,000 files in the main thread is simply out of the question.
But I agree 100% with you on the spaghetti. I'm researching and testing a new model for Code Completion to implement after v1.0 is released. It's a longterm project.
--- End quote ---
I wasn't claiming that you could parse files on the main thread, I was just claiming the threading could be a lot simpler, which would make it more maintainable. the actual parser code, by comparison, looks quite readable.
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version