(kvrocks) branch unstable updated: fix(bloom): invalid access in GetSelf (#2867)

[hulk]

This is an automated email from the ASF dual-hosted git repository.

hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 89991ef86 fix(bloom): invalid access in GetSelf (#2867)
89991ef86 is described below

commit 89991ef86b1aba7b3cdf98ef7b6c5f707fef1c66
Author: mwish <maplewish...@gmail.com>
AuthorDate: Fri Apr 11 14:25:24 2025 +0800

    fix(bloom): invalid access in GetSelf (#2867)
---
 src/types/redis_bloom_chain.cc          |  4 +++-
 tests/cppunit/types/bloom_chain_test.cc | 34 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/src/types/redis_bloom_chain.cc b/src/types/redis_bloom_chain.cc
index cc2e45a23..c2c512987 100644
--- a/src/types/redis_bloom_chain.cc
+++ b/src/types/redis_bloom_chain.cc
@@ -204,7 +204,9 @@ rocksdb::Status BloomChain::InsertCommon(engine::Context 
&ctx, const Slice &user
         return slice;
       };
       auto strip_string_from_pinnable_slice = [](rocksdb::PinnableSlice 
&slice) -> std::string {
-        if (slice.GetSelf() != nullptr) {
+        if (!slice.IsPinned()) {
+          // Only a "PinSelf" slice ( which is !IsPinned )
+          // can operate in this way.
           return std::move(*slice.GetSelf());
         }
         return slice.ToString();
diff --git a/tests/cppunit/types/bloom_chain_test.cc 
b/tests/cppunit/types/bloom_chain_test.cc
index cace78d45..f2f022291 100644
--- a/tests/cppunit/types/bloom_chain_test.cc
+++ b/tests/cppunit/types/bloom_chain_test.cc
@@ -21,6 +21,7 @@
 #include <gtest/gtest.h>
 
 #include <memory>
+#include <random>
 
 #include "test_base.h"
 #include "types/redis_bloom_chain.h"
@@ -111,3 +112,36 @@ TEST_F(RedisBloomChainTest, DuplicateInsert) {
   s = sb_chain_->MAdd(*ctx_, key_, arrays, &results);
   EXPECT_TRUE(s.ok());
 }
+
+TEST_F(RedisBloomChainTest, MultiThreadInsert) {
+  // Concurrent insert, every task would insert 100 random values
+  std::mutex mu;
+
+  std::default_random_engine gen(42);
+  std::uniform_int_distribution<uint32_t> dist;
+  auto insert_task = [&]() {
+    // Generate 100 random keys
+    std::vector<std::string> arrays;
+    for (size_t idx = 0; idx < 100; ++idx) {
+      arrays.push_back("itemx" + std::to_string(dist(gen)));
+    }
+    engine::Context ctx(storage_.get());
+    // Call madd
+    std::vector<redis::BloomFilterAddResult> results;
+    results.resize(arrays.size());
+    auto s = sb_chain_->MAdd(ctx, key_, arrays, &results);
+    EXPECT_TRUE(s.ok());
+  };
+
+  std::vector<std::thread> threads;
+  threads.reserve(10);
+  for (int32_t thread_idx = 0; thread_idx < 10; ++thread_idx) {
+    threads.emplace_back([&]() {
+      std::lock_guard<std::mutex> lock(mu);
+      insert_task();
+    });
+  }
+  for (auto& thread : threads) {
+    thread.join();
+  }
+}