Re: [PATCH] Bug #2583 fix: pure virtual method called

Alex Rousskov wrote:
> Bug #2583 fix: pure virtual method called
>
> When a cbdata-protected class holds its own cbdata and has virtual
> toCbdata(), there is a catch22 problem: we need cbdata to know whether
> the pointer to the class object is valid, and we need to dereference
> that pointer to get cbdata.
>
> Added CbcPointer class to hold both a pointer to a potentially freed
> class object and the cbdata pointer protecting that object. Keeping the
> cbdata pointer allows us to test whether the object is still there
> without dereferencing the object pointer.
>
> Use the CbcPointer class to hold safe pointers to AsyncJobs. This
> prevents "pure virtual method called" failures because we no longer
> dereference freed job pointers.
>
> Removed Initiator parameter from many initiatee constructors. The
> Adaptation::Initiator::initiateAdaptation method now sets the initiator
> of the job. This makes the constructor profile simpler and removes the
> need to propagate Initiator changes through all the [nested] constructors.
>
> Renamed AsyncJob::AsyncStart() to AsyncJob::Start(). I had to change the
> callers code anyway and it was a good opportunity to remove the
> redundant "Async".
>
>
> Special thanks to Stefan Fritsch for updating and testing an earlier
> version of this patch.
>
>
> -------------------
>
> P.S. The underlying changes have been tested in production but I had to
> polish them before submitting to trunk. Some advanced C++ concepts might
> not compile on old/broken compilers. We will try to remove/change them
> if needed.
>
> P.P.S. This change is required for at least one more bug fix and for the
> the 1xx handling patch.
>

One problem:

src/base/CbcPointer.h:
  * #include "TextException.h" needs to be "base/TextException.h".
NP: this should be failing on build since -I. is undefined in src/.

The rest is just more polish

Several of the files with added #include entries:

  * For clarity as they grow its useful to keep the #include statements
alphabetical. The old code remains messy, but no need to mess the nice
clean new code. :)
  * If this is for order-dependent includes that is a violation of the
testHeaders dependency guarantee in all src/* .h code.

Ditto for *_SOURCES list in src/base/Makefile.am.

src/CommCalls.h:
  * useless whitespace addition.

Other than that +1.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.6
   Beta testers wanted for 3.2.0.1