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

Revision history for this message
Monty Taylor (mordred) 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-----

« Back to merge proposal