Re: [PATCH 4/8] reconfiguration leaks: Cache Manager menu items

2014-06-01 Thread Amos Jeffries
On 25/04/2014 7:55 p.m., Amos Jeffries wrote:
> On 25/04/2014 12:50 p.m., Alex Rousskov wrote:
>> Do not register the same Cache Manager action more than once, to avoid
>> wrong mgr:menu output and the impression of a reconfigure memory leak.
>>
>> The old code was comparing action object pointers, which could not work,
>> and was adding the same action on every reconfigure call for modules
>> that register with Cache Manager during [re]configuration.
>>
>> We already have a working method for finding registered actions. Use it.
>>
>> Alex.
>>
> 
> Ouch. I thought we had this is trunk a while back...
> 
> +1 if it still needs to go in.
> 
> Amos
> 

Applied as trunk rev.13433

Amos


Re: [PATCH 4/8] reconfiguration leaks: Cache Manager menu items

2014-04-25 Thread Amos Jeffries
On 25/04/2014 12:50 p.m., Alex Rousskov wrote:
> Do not register the same Cache Manager action more than once, to avoid
> wrong mgr:menu output and the impression of a reconfigure memory leak.
> 
> The old code was comparing action object pointers, which could not work,
> and was adding the same action on every reconfigure call for modules
> that register with Cache Manager during [re]configuration.
> 
> We already have a working method for finding registered actions. Use it.
> 
> Alex.
> 

Ouch. I thought we had this is trunk a while back...

+1 if it still needs to go in.

Amos


[PATCH 4/8] reconfiguration leaks: Cache Manager menu items

2014-04-24 Thread Alex Rousskov
Do not register the same Cache Manager action more than once, to avoid
wrong mgr:menu output and the impression of a reconfigure memory leak.

The old code was comparing action object pointers, which could not work,
and was adding the same action on every reconfigure call for modules
that register with Cache Manager during [re]configuration.

We already have a working method for finding registered actions. Use it.

Alex.

Do not register the same Cache Manager action more than once,
to avoid wrong mgr:menu output and the impression of a reconfigure memory leak.

The old code was comparing action object pointers, which could not work, and
was adding the same action on every reconfigure call for modules that
register with Cache Manager during [re]configuration. 

We already have a working method for finding registered actions. Use it.

=== modified file 'src/cache_manager.cc'
--- src/cache_manager.cc	2013-10-25 00:13:46 +
+++ src/cache_manager.cc	2014-04-24 20:12:30 +
@@ -65,41 +65,41 @@ class ClassActionCreator: public Mgr::Ac
 {
 public:
 typedef Mgr::Action::Pointer Handler(const Mgr::Command::Pointer &cmd);
 
 public:
 ClassActionCreator(Handler *aHandler): handler(aHandler) {}
 
 virtual Mgr::Action::Pointer create(const Mgr::Command::Pointer &cmd) const {
 return handler(cmd);
 }
 
 private:
 Handler *handler;
 };
 
 /// Registers new profiles, ignoring attempts to register a duplicate
 void
 CacheManager::registerProfile(const Mgr::ActionProfile::Pointer &profile)
 {
 Must(profile != NULL);
-if (std::find(menu_.begin(), menu_.end(), profile) == menu_.end()) {
+if (!CacheManager::findAction(profile->name)) {
 menu_.push_back(profile);
 debugs(16, 3, HERE << "registered profile: " << *profile);
 } else {
 debugs(16, 2, HERE << "skipped duplicate profile: " << *profile);
 }
 }
 
 /**
  \ingroup CacheManagerAPI
  * Registers a C-style action, which is implemented as a pointer to a function
  * taking as argument a pointer to a StoreEntry and returning void.
  * Implemented via CacheManagerActionLegacy.
  */
 void
 CacheManager::registerProfile(char const * action, char const * desc, OBJH * handler, int pw_req_flag, int atomic)
 {
 debugs(16, 3, HERE << "registering legacy " << action);
 const Mgr::ActionProfile::Pointer profile = new Mgr::ActionProfile(action,
 desc, pw_req_flag, atomic, new Mgr::FunActionCreator(handler));
 registerProfile(profile);