0

We have our own smart pointers class which is reference counted using basic AddRef and Release.

While debugging I am able to see lot of objects not being released properly. I can see which objects not being released but its hard to find out from which place they get allocated.

Is there any good design pattern to address this problem?

I have put debug statements and ADDREF and RELEASE and I can see reference count when release is called . In some cases its more than 1, that means pointer is not deleted.

If I start putting breakpoints on shared_ptr const , then it will called hundred of times and its difficult to pinpoint memory leak.

It looks like some cyclic references getting created which is causing these leaks.

anand
  • 11,071
  • 28
  • 101
  • 159
  • How are you detecting that things are not being released properly? Which IDE are you using? MSVC has a quick and dirty `_CrtSetBreakAlloc` which might help if you can reproduce the problem deterministically. – tenfour Oct 09 '12 at 17:42
  • If you have cyclic data structures then don't use reference counting. There's no way to fix reference counting to take account of pointer cycles. – john Oct 09 '12 at 17:54
  • Did you implement the copy constructor and assignment correctly? – Kerrek SB Oct 09 '12 at 17:54
  • why not use standard smart pointers? – pm100 Oct 09 '12 at 17:55
  • If RAII is implemented correctly (first thing to check), multithreading issues can break reference counting if the counter is not managed atomically. The best is to use `{std[::tr1]|boost}::shared_ptr` (or ATL smart pointers if you're using this as a means of implementing IUnknown). But you probably know that anyway. – Alexandre C. Oct 09 '12 at 18:01

2 Answers2

2

Don't use design patterns for this. Don't add code bloat just to detect leaks.

Use a memory tool. Valgrind comes to mind, and a number of commercial ones for Windows.

Also...

I can see which objects not being released but its hard to find out from which place they get allocated.

Why? Set a breakpoint in the constructor of the object. Or use the standard pointers instead (std::shared_ptr, std::unique_ptr).

As per your edit:

I have put debug statements and ADDREF and RELEASE and I can see reference count when release is called . In some cases its more than 1, that means pointer is not deleted.

Are you taking into account copy elision (RVO/NRVO)? This seems more likely to be the cause.

Community
  • 1
  • 1
Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • You know, I almost never respond to questions in which you have already responded because I generally agree with you 100%. Not this time. – John Dibling Oct 09 '12 at 17:48
  • @JohnDibling :) Well, in that case, I look forward to the answer (please ping me when you've added one) – Luchian Grigore Oct 09 '12 at 17:51
  • Posted. The OP's edits have changed the scenario somewhat, but hey -- I spent like 10 minutes typing that. Anyway you get +1 from me. – John Dibling Oct 09 '12 at 18:24
1

Years ago, before auto_ptr, I wrote my own smart pointer class which we used in our codebase extensively. Of course, it was chock full of bugs.

I had exactly the same problem. While I can't show you the actual code I used to identify the call sites where resource leaks originated (because the code no longer exists), I can tell you what I did.

The first thing was I wrote a #define macro >SHUDDER<, similar to the one I outline here, which I could use in place of new to allocate my objects. The macro sent additional parameters to the smart pointer, indicating file & line number of the call site. The smart pointer would retain this.

Then I replaced the global new with my macro when I uttered a magical incantation of #defines turning this functionality on. Obviously, this should be disabled in all Releasse builds, and only used in Debug builds when you're actually debugging this.

Then I let my program run until I got to a point where I wanted to see all the call sites. I'd execute a dump_call_sites() method on the smart pointer there, which dumped the static vector of call site strings to the stdout. Problem hence found, I'd revert all those magical incantations to turn off all this hokus pokus.

This is truly hack-n-slash codiing. It' far from elegant, and it introduces it's own set of problems. But along with printf debugging, it has its place. If you don't want to or can't go to the effort of loading up your Rational Purify or Bounds Checker type product, this could help you quickly identify where your leaks are coming from.

Community
  • 1
  • 1
John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • Fair alternative, but I prefer using existing tools. This just seems to add a lot of code bloat for something that can be identified using other means. But I do get the tendency to write one's own tools and mechanisms. :) – Luchian Grigore Oct 09 '12 at 18:28
  • I liked it not because it was invented by me, but becasue it was a heck of a lot easier to turn on and run than loading up Rational Purify and wait 3 days for my program to run. – John Dibling Oct 09 '12 at 18:31
  • 1
    Another option would be some debugger scripts. Put break points in the relevant functions, attach scripts that grabs the relevant info from the call stack (file/line of calling function, etc.) and prints it out. No code bloat directly in the project but the work/scripts/breakpoints can be easily shared with the project. – bames53 Oct 09 '12 at 18:33
  • Purify is a pain indeed (but if I recall correctly, it caches instrumented binaries, so leaving it overnight probably would have done it and fixed later runs). – Luchian Grigore Oct 09 '12 at 18:33
  • @bames53: I'd like to subscribe to your newsletter. – John Dibling Oct 09 '12 at 18:35
  • @blame53: can u please provide some sample script or some link giving information about debugger scripts – anand Oct 10 '12 at 18:29