From cb74b048afa851fef5ed8b09ad593d954a3eefd2 Mon Sep 17 00:00:00 2001
From: praveenbingo <praveen@dremio.com>
Date: Thu, 23 Aug 2018 20:36:03 +0530
Subject: [PATCH] GDV-31:[Java][C++]Fixed concurrency issue in cache. (#84)

* GDV-31:[Java][C++]Fixed concurrency issue in cache.

Modifications were happening in get without a mutex.
Wrote a test to verify and prevent regression.
---
 cpp/src/gandiva/codegen/cache.h      | 8 ++------
 cpp/src/gandiva/codegen/filter.cc    | 4 ++--
 cpp/src/gandiva/codegen/projector.cc | 4 ++--
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/cpp/src/gandiva/codegen/cache.h b/cpp/src/gandiva/codegen/cache.h
index b919ff17f361e..2aa8354a5dd7c 100644
--- a/cpp/src/gandiva/codegen/cache.h
+++ b/cpp/src/gandiva/codegen/cache.h
@@ -25,19 +25,15 @@ template <class KeyType, typename ValueType>
 class Cache {
  public:
   Cache(size_t capacity = CACHE_SIZE) : cache_(capacity) {}
-  ValueType GetCachedModule(KeyType cache_key) {
+  ValueType GetModule(KeyType cache_key) {
     boost::optional<ValueType> result;
-    result = cache_.get(cache_key);
-    if (result != boost::none) {
-      return result.value();
-    }
     mtx_.lock();
     result = cache_.get(cache_key);
     mtx_.unlock();
     return result != boost::none ? result.value() : nullptr;
   }
 
-  void CacheModule(KeyType cache_key, ValueType module) {
+  void PutModule(KeyType cache_key, ValueType module) {
     mtx_.lock();
     cache_.insert(cache_key, module);
     mtx_.unlock();
diff --git a/cpp/src/gandiva/codegen/filter.cc b/cpp/src/gandiva/codegen/filter.cc
index 52cf53075e356..b9255d98c1c9e 100644
--- a/cpp/src/gandiva/codegen/filter.cc
+++ b/cpp/src/gandiva/codegen/filter.cc
@@ -46,7 +46,7 @@ Status Filter::Make(SchemaPtr schema, ConditionPtr condition,
                                   Status::Invalid("configuration cannot be null"));
   static Cache<FilterCacheKey, std::shared_ptr<Filter>> cache;
   FilterCacheKey cacheKey(schema, configuration, *(condition.get()));
-  std::shared_ptr<Filter> cachedFilter = cache.GetCachedModule(cacheKey);
+  std::shared_ptr<Filter> cachedFilter = cache.GetModule(cacheKey);
   if (cachedFilter != nullptr) {
     *filter = cachedFilter;
     return Status::OK();
@@ -67,7 +67,7 @@ Status Filter::Make(SchemaPtr schema, ConditionPtr condition,
 
   // Instantiate the filter with the completely built llvm generator
   *filter = std::make_shared<Filter>(std::move(llvm_gen), schema, configuration);
-  cache.CacheModule(cacheKey, *filter);
+  cache.PutModule(cacheKey, *filter);
   return Status::OK();
 }
 
diff --git a/cpp/src/gandiva/codegen/projector.cc b/cpp/src/gandiva/codegen/projector.cc
index d1f20c9213553..598da05368e2e 100644
--- a/cpp/src/gandiva/codegen/projector.cc
+++ b/cpp/src/gandiva/codegen/projector.cc
@@ -53,7 +53,7 @@ Status Projector::Make(SchemaPtr schema, const ExpressionVector &exprs,
   // see if equivalent projector was already built
   static Cache<ProjectorCacheKey, std::shared_ptr<Projector>> cache;
   ProjectorCacheKey cache_key(schema, configuration, exprs);
-  std::shared_ptr<Projector> cached_projector = cache.GetCachedModule(cache_key);
+  std::shared_ptr<Projector> cached_projector = cache.GetModule(cache_key);
   if (cached_projector != nullptr) {
     *projector = cached_projector;
     return Status::OK();
@@ -84,7 +84,7 @@ Status Projector::Make(SchemaPtr schema, const ExpressionVector &exprs,
   // Instantiate the projector with the completely built llvm generator
   *projector = std::shared_ptr<Projector>(
       new Projector(std::move(llvm_gen), schema, output_fields, configuration));
-  cache.CacheModule(cache_key, *projector);
+  cache.PutModule(cache_key, *projector);
   return Status::OK();
 }