Code review comment for lp:~jaypipes/drizzle/bugs

Oh, it's the PCRE library terminology, that's all. The "match vector"
is an address of the set of integer offsets that you pass to pcre_exec().

Monty Taylor wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Looks good to me in general. Why aren't you using a std::vector for
> match vector?
>
> Jay Pipes wrote:
>> Jay Pipes has proposed merging lp:~jaypipes/drizzle/bugs into lp:drizzle.
>>
>> Requested reviews:
>> Brian Aker (brianaker)
>> Drizzle-developers (drizzle-developers)
>> Related bugs:
>> #384531 compile failure centos 5.2
>> https://bugs.launchpad.net/bugs/384531
>> #402831 Query Logging output missing a comma
>> https://bugs.launchpad.net/bugs/402831
>> #402855 session->connect_utime doesn't appear to be populated
>> https://bugs.launchpad.net/bugs/402855
>> #409711 plugin/command_log/command_log.cc can't be compiled on 32 bit platforms
>> https://bugs.launchpad.net/bugs/409711
>> #434128 Build failure on debian
>> https://bugs.launchpad.net/bugs/434128
>>
>>
>>
>>
>> Fixes LP Bug #500031:
>>
>> "dbt2 fails with 1024 connections"
>>
>> After investigation into this, I discovered that there was a
>> race condition in TemporalFormat::match():
>>
>> TemporalFormat::_re is the compiled PCRE regular expression object
>> inside each of the TemporalFormat objects, which are shared
>> among all threads and live in global scope.
>>
>> Unfortunately, TemporalFormat::match() was using the member
>> variable TemporalFormat::_match_vector as its match state.
>> At high concurrency, this means that the following race
>> condition could happen:
>>
>> Thread 1 executes pcre_exec() and finds a match, therefore
>> populating TemporalFormat::_match_vector of integers
>> with the position offsets of the matched pieces of the temporal object.
>>
>> Thread 1, during construction of the Temporal output of
>> TemporalFormat::match(), uses these _match_vector position
>> offsets in calling std::string::substr on a copy of the
>> matched string, essentially "cutting up" the string
>> into year, month, day, etc.
>>
>> Thread 2 executes pcre_exec() and also finds a match,
>> thereby changing TemporalFormat::_match_vector to something
>> different
>>
>> Thread 1 continues trying to use std::string::substr(),
>> but now uses offsets that are invalid for its string,
>> thereby producing an out_of_range exception.
>>
>> The solution is to pull the TemporalFormat::_match_vector
>> member variable and instead put a function-scope-level
>> match_vector variable on the stack inside TemporalFormat::match().
>>
>>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iEYEARECAAYFAksz8OcACgkQ2Jv7/VK1RgGUHQCeIYj2o9RCC7rIbG+GLXyZqOmN
> 0tEAn1IV+nwIJszqCqtimktphYi01ymm
> =oJvO
> -----END PGP SIGNATURE-----