Re: Possible reason for std::vector crashes

Hi,
  I think I have isolated the changelet in FwdState::connectionList.
However WHAT in that changelet may be causing the crashes is still
unknown. The hunt for iterate-after-clear has so far turned up
nothing.

On Wed, Mar 26, 2014 at 8:01 AM, Niki Gorchilov <niki_at_gorchilov.com> wrote:
> Hello there,
>
> Is there any progress regarding the random coredumps related to
> std::vector migration?
>
> Best,
> Niki
>
> On Mon, Mar 17, 2014 at 6:41 PM, Kinkie <gkinkie_at_gmail.com> wrote:
>> Yes, I will.
>>
>> On Mon, Mar 17, 2014 at 5:24 PM, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>> Hello,
>>>
>>> I have discovered one difference between std::vector and Squid's own
>>> Vector implementation that might explain some of the random crashes that
>>> some of us have been seeing after the Vector class was replaced with
>>> std::vector in trunk.
>>>
>>> Squid Vector sets the number of stored elements to zero on destruction.
>>> Std::vector does not do that. Here is the output of a simple test code
>>> quoted in the postscriptum:
>>>
>>>> alive std::vector size: 1
>>>> deleted std::vector size: 3
>>>> alive Squid Vector size: 1
>>>> deleted Squid Vector size: 0
>>>
>>> Both vectors behave correctly, but std::vector code is optimized not to
>>> do extra work such as maintaining the size member. This means that
>>> iterating a deleted Squid Vector is often safe (until the freed memory
>>> is overwritten) because the broken caller code would just discover that
>>> there are no items in the vector and move on.
>>>
>>> Iterating the deleted std::vector usually leads to crashes because all
>>> iteration-related methods such as size() rely on immediately deleted and
>>> changed pointers (std::vector does not store its size as a data member
>>> but uses memory pointers to compute the number of stored elements).
>>>
>>> This difference leads to std::vector migration problems such as this
>>> trivially reproducible one:
>>>
>>>> Adaptation::AccessRules &
>>>> Adaptation::AllRules()
>>>> {
>>>> static AccessRules TheRules;
>>>> return TheRules;
>>>> }
>>>
>>> After TheRules array is deallocated during global destructions,
>>> iterating AllRules() becomes unsafe. The old Vector-based code usually
>>> worked because deallocated TheRules had zero size.
>>>
>>> The specific bug mentioned above is trivial to work around by allocating
>>> TheRules dynamically (and never deleting it). However, if similar bugs
>>> exist in other code where Vector is used as a data member of some
>>> transaction-related structure, then they will lead to crashes. It is
>>> quite possible that the affected structure's memory itself is never
>>> deleted when the offending code accesses it (due to cbdata locks, for
>>> example) so the [equally buggy] Vector-based code "works".
>>>
>>> One way we can try to catch such cases is by temporary switching back to
>>> Vector and then:
>>>
>>> * Modifying Vector to mark deleted Vector objects:
>>>
>>> Vector::~Vector() {
>>> clean();
>>> deleted = true;
>>> }
>>>
>>> * And adjusting *every* Vector method to assert if a deleted object is
>>> still being used. For example:
>>>
>>> template<class E>
>>> size_t
>>> Vector<E>::size() const
>>> {
>>> assert(!deleted);
>>> return count;
>>> }
>>>
>>> If one of those asserts is triggered, we would be closer to solving this
>>> mystery.
>>>
>>>
>>> Kinkie, if you agree with this analysis, would you be able to make the
>>> above modifications and test?
>>>
>>>
>>> Thank you,
>>>
>>> Alex.
>>>
>>>
>>> Goes into Squid's main.cc:
>>>> +#include <vector>
>>>> +#include "base/Vector.h"
>>>> int
>>>> main(int argc, char **argv)
>>>> {
>>>> + typedef std::vector<int> StdVector;
>>>> + StdVector *stdv = new StdVector;
>>>> + stdv->push_back(1);
>>>> + std::cout << "alive std::vector size: " << stdv->size() << "\n";
>>>> + delete stdv;
>>>> + std::cout << "deleted std::vector size: " << stdv->size() << "\n";
>>>> +
>>>> + typedef Vector<int> SqdVector;
>>>> + SqdVector *sqdv = new SqdVector;
>>>> + sqdv->push_back(1);
>>>> + std::cout << "alive Squid Vector size: " << sqdv->size() << "\n";
>>>> + delete sqdv;
>>>> + std::cout << "deleted Squid Vector size: " << sqdv->size() << "\n";
>>>> + if (true) return 0;
>>>> return SquidMainSafe(argc, argv);
>>>> }
>>>
>>
>>
>>
>> --
>> Francesco

-- 
    Francesco