On 01/07/2014 08:35 AM, Alex Rousskov wrote:
> 2) [Add cbdata debugging to] find which object stores the invalid cbdata
> pointer. Then find how that invalid pointer gets to that object.
I think it happens here:
>> #8 0x0834f190 in CommCbMemFunT (aMeth=
>> (void (Comm::TcpReceiver::*)(Comm::TcpReceiver * const, const
>> CommCbMemFunT<Comm::TcpReceiver, CommCloseCbParams>::Params &))
>> 0x834e270 <Comm::TcpReceiver::tcpConnectionClosed(CommCloseCbParams
>> const&)>, aJob=..., this=0xbffff268) at ../../../src/CommCalls.h:180
where we are passing a raw job pointer (job.get()) to CommCloseCbParams
object which then uses it as cbdata in CommCommonCbParams constructor:
> CommCbMemFunT(const CbcPointer<C> &job, Method meth): JobDialer<C>(job),
> CommDialerParamsT<Params_>(job.get()),
> CommCloseCbParams(void *aData);
> CommCommonCbParams::CommCommonCbParams(void *aData):
> data(cbdataReference(aData)), ...
Does the attached untested patch help?
FWIW, you may also use the attached test program as a simple test case.
It has the same class hierarchy as yours and illustrates how while
individual objects have different "this" pointers, they all have the
same (and correct) toCbdata() pointer, regardless of the inheritance order:
BC: TrueCbdata=0xbfb306f8
A=0xbfb3070c A::cbdata=0xbfb306f8 OK
B=0xbfb306f8 B::cbdata=0xbfb306f8 OK
C=0xbfb30700 C::cbdata=0xbfb306f8 OK
BC=0xbfb306f8 BC::cbdata=0xbfb306f8 OK
CB: TrueCbdata=0xbfb30714
A=0xbfb30728 A::cbdata=0xbfb30714 OK
B=0xbfb3071c B::cbdata=0xbfb30714 OK
C=0xbfb30714 C::cbdata=0xbfb30714 OK
CB=0xbfb30714 CB::cbdata=0xbfb30714 OK
The buggy code in Squid was using a raw job pointer from the left column
instead of using toCbdata() pointer from the right column.
Unfortunately, due to cbdata being passed and stored as void*, the
compiler cannot catch such bugs. There may be more of these.
HTH,
Alex.
Comm job callbacks need job's cbdata pointer, not a job pointer.
Otherwise, in complex inheritance hierarchies, some [inner] classes will
hit cbdata cookie assertions when scheduling Comm calls with callbacks.
=== modified file 'src/CommCalls.h'
--- src/CommCalls.h 2013-10-25 00:13:46 +0000
+++ src/CommCalls.h 2014-01-14 02:12:00 +0000
@@ -159,41 +159,41 @@ public:
// Get comm params of an async comm call
template <class Params>
Params &GetCommParams(AsyncCall::Pointer &call)
{
typedef CommDialerParamsT<Params> DialerParams;
DialerParams *dp = dynamic_cast<DialerParams*>(call->getDialer());
assert(dp);
return dp->params;
}
// All job dialers with comm parameters are merged into one since they
// all have exactly one callback argument and differ in Params type only
template <class C, class Params_>
class CommCbMemFunT: public JobDialer<C>, public CommDialerParamsT<Params_>
{
public:
typedef Params_ Params;
typedef void (C::*Method)(const Params &io);
CommCbMemFunT(const CbcPointer<C> &aJob, Method aMeth): JobDialer<C>(aJob),
- CommDialerParamsT<Params_>(aJob.get()),
+ CommDialerParamsT<Params_>(aJob->toCbdata()),
method(aMeth) {}
virtual bool canDial(AsyncCall &c) {
return JobDialer<C>::canDial(c) &&
this->params.syncWithComm();
}
virtual void print(std::ostream &os) const {
os << '(';
this->params.print(os);
os << ')';
}
public:
Method method;
protected:
virtual void doDial() { ((&(*this->job))->*method)(this->params); }
};
