HomerReid / scuff-em

A comprehensive and full-featured computational physics suite for boundary-element analysis of electromagnetic scattering, fluctuation-induced phenomena (Casimir forces and radiative heat transfer), nanophotonics, RF device engineering, electrostatics, and more. Includes a core library with C++ and python APIs as well as many command-line applications.
http://www.homerreid.com/scuff-em
GNU General Public License v2.0
126 stars 50 forks source link

Delete linked list of sources in IncField destructor? #51

Closed odmiller closed 9 years ago

odmiller commented 9 years ago

I think the Next member variable of an IncField needs to be deleted (recursively) in the IncField destructor, something like:

if (Next)
    ~Next();

Otherwise, only the first IncField in the list will be freed from memory.

HomerReid commented 9 years ago

Thanks for this suggestion. I started to implement it, but then recalled an old doctrine from my C training that says "every individual malloc() should be paired with an individual free()." I don't know if this dictum carries over to the C++ world, but the naive C++ extension of this would say "every individual new should be paired with an individual delete" which would be violated by the proposed extension.

The concern is that coder A might somewhere instantiate a GaussianBeam which subsequently gets chained onto coder B's PlaneWave without coder A necessarily being aware of it. When coder B deletes her PlaneWave, coder A's GaussianBeam gets destroyed along with it, but coder A doesn't know this, and may attempt to access the GaussianBeam after it has been destroyed.

I wonder if the C++ world has developed a canonical protocol for this type of situation?

In the meantime, my instinct is to keep the IncField destructor as is, but instead to add to libIncField a non-class-member function void DeleteIncFieldChain(IncField *IF) which does what you are suggesting. I have added this function to 598322988ce5bebab90e410ce9dced6e34140b7f. Simple tests seemed to work, but let me know what you think.

odmiller commented 9 years ago

Your solution works well for me - thanks!

I would say that a destructor is not quite the same thing as a delete (or a free), but I do see the freedom you want to preserve (to delete an IncField from a chain without deleting the whole chain). I think the ambiguity comes from the dual role of an IncField as both an object and as implementing a subset of the functionality of a linked list (whereas for example the list in the c++ STL has separate functions for list destruction and destroying/inserting single objects).