You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jason Huynh <hu...@gmail.com> on 2016/02/11 00:28:43 UTC

Re: Review Request 43449: GEODE-890: CompiledLike member variables are not thread safe

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43449/
-----------------------------------------------------------

(Updated Feb. 10, 2016, 11:28 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, and Dan Smith.


Changes
-------

Fixing typo again.  Apparently I like to type GEDOE instead of GEODE


Summary (updated)
-----------------

GEODE-890: CompiledLike member variables are not thread safe


Repository: geode


Description
-------

If a compiled like query is used concurrently, for example a bind parameter query from a client, the member variables are not thread safe and can possibly lead to a stack over flow.
This is because the CompiledLike is unique in that it creates new junctions when executing and if one of the member variables is set to true at a certain point, can lead to an infinite recursion.

Removed some dead code from a testcase that I was thinking of extending (which I didn't end up extending)
Minor refactor of QueryExecutionContext
Added tests for QueryExecutionContext
Added a MultithreadedTester class that helped with recreating the issue.


Diffs
-----

  gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledLike.java 6abcd62 
  gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/ExecutionContext.java 08212ea 
  gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContext.java b8b88b2 
  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/MultithreadedTester.java PRE-CREATION 
  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/HelperTestCase.java f4132da 
  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/ParameterBindingJUnitTest.java 9b34aa9 
  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QCompilerJUnitTest.java 2510c36 
  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContextJUnitTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/43449/diff/


Testing
-------


Thanks,

Jason Huynh


Re: Review Request 43449: GEODE-890: CompiledLike member variables are not thread safe

Posted by Jason Huynh <hu...@gmail.com>.

> On Feb. 12, 2016, 1:25 a.m., anilkumar gingade wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledLike.java, line 79
> > <https://reviews.apache.org/r/43449/diff/1/?file=1239916#file1239916line79>
> >
> >     Will it be a good idea to use Integer.valueOf(-1)

Autoboxing will be very similar and the I think the code is a little more readable this way...


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43449/#review118901
-----------------------------------------------------------


On Feb. 10, 2016, 11:28 p.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43449/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 11:28 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> If a compiled like query is used concurrently, for example a bind parameter query from a client, the member variables are not thread safe and can possibly lead to a stack over flow.
> This is because the CompiledLike is unique in that it creates new junctions when executing and if one of the member variables is set to true at a certain point, can lead to an infinite recursion.
> 
> Removed some dead code from a testcase that I was thinking of extending (which I didn't end up extending)
> Minor refactor of QueryExecutionContext
> Added tests for QueryExecutionContext
> Added a MultithreadedTester class that helped with recreating the issue.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledLike.java 6abcd62 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/ExecutionContext.java 08212ea 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContext.java b8b88b2 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/MultithreadedTester.java PRE-CREATION 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/HelperTestCase.java f4132da 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/ParameterBindingJUnitTest.java 9b34aa9 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QCompilerJUnitTest.java 2510c36 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContextJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43449/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>


Re: Review Request 43449: GEODE-890: CompiledLike member variables are not thread safe

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43449/#review118901
-----------------------------------------------------------


Fix it, then Ship it!




Ship It!


gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledLike.java (line 79)
<https://reviews.apache.org/r/43449/#comment180214>

    Will it be a good idea to use Integer.valueOf(-1)


- anilkumar gingade


On Feb. 10, 2016, 11:28 p.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43449/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 11:28 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> If a compiled like query is used concurrently, for example a bind parameter query from a client, the member variables are not thread safe and can possibly lead to a stack over flow.
> This is because the CompiledLike is unique in that it creates new junctions when executing and if one of the member variables is set to true at a certain point, can lead to an infinite recursion.
> 
> Removed some dead code from a testcase that I was thinking of extending (which I didn't end up extending)
> Minor refactor of QueryExecutionContext
> Added tests for QueryExecutionContext
> Added a MultithreadedTester class that helped with recreating the issue.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CompiledLike.java 6abcd62 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/ExecutionContext.java 08212ea 
>   gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContext.java b8b88b2 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/MultithreadedTester.java PRE-CREATION 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/dunit/HelperTestCase.java f4132da 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/ParameterBindingJUnitTest.java 9b34aa9 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QCompilerJUnitTest.java 2510c36 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/internal/QueryExecutionContextJUnitTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43449/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>