----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52805/#review152401 -----------------------------------------------------------
Ship it! Ship It! - nabarun nag On Oct. 12, 2016, 8:58 p.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52805/ > ----------------------------------------------------------- > > (Updated Oct. 12, 2016, 8:58 p.m.) > > > Review request for geode, Barry Oglesby and nabarun nag. > > > Repository: geode > > > Description > ------- > > When executing a function from a client, we can be adding results to the > result collector from multiple threads. Our docs claim the user should > not have to synchronize their result collector. One code path was already > synchronizing on the collector when adding results. However, if the > function returned an exception we were not synchronizing. > > Adding a SynchronizedResultCollector and wrapping the users collector in > that to ensure that there will be no unsynchronized access of the result > collector. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteFunctionOp.java > 6597b680227fb47492dc973baecffd504d6cdf68 > > geode-core/src/main/java/org/apache/geode/cache/client/internal/ExecuteRegionFunctionSingleHopOp.java > 51ea8e4fbc64acbbd1165856a2dd09704d63e857 > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/ServerFunctionExecutor.java > 4295898583aff1409bf6acdd84c5a4c8a8709e51 > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/ServerRegionFunctionExecutor.java > b5bc684e0b29bfc62dd958ccff49d47c2bad36fa > > geode-core/src/main/java/org/apache/geode/internal/cache/execute/util/SynchronizedResultCollector.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/52805/diff/ > > > Testing > ------- > > Ran the affected test 500 times with this fix. It failed 11 times without it, > passed with the fix. > > > Thanks, > > Dan Smith > >