On 04/09/2014 02:17 AM, Alex Rousskov wrote: > On 04/08/2014 09:28 AM, Tsantilas Christos wrote: >> The global objects in C++ normally destroyed in the reverse order they >> created. The Adaptation::*::TheConfig objects created on squid start, >> but the std::vector lists created later when the >> adaptation::All(Rules|Services|Groups)() method called. On squid exit, >> the std::vector lists destroyed first. >> >> I am attaching a patch which solves this problem. This patch creates >> small temporary object classes to store the std::vector lists for >> Rules, Services and Groups, which are responsible to empty and release >> the lists on shutdown. >> These lists are not emptied from Adaptation::Config >> (Adaptation::*::TheConfig objects) any more, and a new method the >> Adaptation::Config::Clear() added to empty these lists on reconfigure. > > I am not objecting to your change, but please consider the attached > patch as a much simpler alternative that avoids static destruction > dependency altogether. The proposed commit message is in the patch preamble.
I agree that your patch is simpler, and better. What still I do not like in adaptation::Config code is to release inside adaptation::Config (adaptation/Config.cc) objects stored in adaptation/AccessRules.cc for example. But I do not believe that worth to spent our time for this now. > > I prefer my version because it is much simpler and because the true/core > problem here is kind of elsewhere: We just do not cleanup properly on > exit(*). This is why reconfiguration works without coredumps but exiting > Squid creates problems (kind of). That is a different problem to solve > IMO... OK, I agree. > > > If you want to polish your patch instead, please briefly document the > new classes and methods to comply with code style rules! You should > probably also move most static methods manipulating > groups/rules/services into the new classes. For example, > Adaptation::FindGroup() should be moved into GroupsCfg because it is > specific to adaptation groups, right? > > > Thank you, > > Alex. > (*) There is a bunch of on-exit cleanup code in main.cc that is > currently disabled that should be polished and re-enabled, at least > partially, but we have much bigger problems with memory cleanup during > reconfiguration that we need to address. Patches pending. >
