Skip to content

Commit

Permalink
curvebs(client): Fix segment fault when opening clonesource files con…
Browse files Browse the repository at this point in the history
…currently

Signed-off-by: Hanqing Wu <[email protected]>
  • Loading branch information
wu-hanqing committed Dec 6, 2023
1 parent 4a68847 commit 6d39feb
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 4 deletions.
8 changes: 8 additions & 0 deletions bazel/gmock.BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,11 @@ cc_library(
visibility = ["//visibility:public"],
deps = [":gtest"],
)

# Library that defines the FRIEND_TEST macro.
cc_library(
name = "gtest_prod",
hdrs = ["googletest/include/gtest/gtest_prod.h"],
includes = ["googletest/include"],
visibility = ["//visibility:public"],
)
1 change: 1 addition & 0 deletions src/client/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ cc_library(
"//proto:topology_cc_proto",
"//proto:chunkserver-cc-protos",
# "//src/chunkserver:chunkserver-lib"
"@com_google_googletest//:gtest_prod",
],
linkopts = [
"-luuid"
Expand Down
15 changes: 11 additions & 4 deletions src/client/source_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,22 @@ SourceReader::ReadHandler* SourceReader::GetReadHandler(
instance->GetIOManager4File()->SetDisableStripe();

curve::common::WriteLockGuard wlk(rwLock_);
auto iter = readHandlers_.find(fileName);
if (iter != readHandlers_.end()) {
instance->UnInitialize();
delete instance;

iter->second.lastUsedSec_.store(::time(nullptr),
std::memory_order_relaxed);
return &iter->second;
}

auto res = readHandlers_.emplace(
std::piecewise_construct,
std::forward_as_tuple(fileName),
std::forward_as_tuple(instance, ::time(nullptr), true));

if (res.second == false) {
instance->UnInitialize();
delete instance;
}
CHECK(res.second);

return &res.first->second;
}
Expand Down
4 changes: 4 additions & 0 deletions src/client/source_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#ifndef SRC_CLIENT_SOURCE_READER_H_
#define SRC_CLIENT_SOURCE_READER_H_

#include <gtest/gtest_prod.h>

#include <atomic>
#include <mutex>
#include <string>
Expand Down Expand Up @@ -97,6 +99,8 @@ class SourceReader {
const std::unordered_map<std::string, ReadHandler>& handlers);

private:
FRIEND_TEST(SourceReaderTest, ConcurrentTest);

SourceReader() = default;
~SourceReader();
SourceReader(const SourceReader&);
Expand Down
102 changes: 102 additions & 0 deletions test/client/source_reader_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright (c) 2023 NetEase Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "src/client/source_reader.h"

#include <gtest/gtest.h>

#include "src/client/mds_client.h"
#include "test/client/fake/fakeMDS.h"

namespace curve {
namespace client {

class SourceReaderTest : public ::testing::Test {
protected:
void SetUp() override {
opt_.metaServerOpt.mdsAddrs.push_back(kServerAddr);
opt_.leaseOpt.mdsRefreshTimesPerLease = 1;

ASSERT_EQ(
0, server_.AddService(&service_, brpc::SERVER_DOESNT_OWN_SERVICE));
ASSERT_EQ(0, server_.Start(kServerAddr, nullptr));

mdsclient_ = std::make_shared<MDSClient>();
ASSERT_EQ(0, mdsclient_->Initialize(opt_.metaServerOpt));

PrepareFakeResponse();
}

void TearDown() override {
server_.Stop(0);
server_.Join();
}

void PrepareFakeResponse() {
curve::mds::GetFileInfoResponse* getFileInfoResponse =
new curve::mds::GetFileInfoResponse();
auto* info = getFileInfoResponse->mutable_fileinfo();
info->set_id(1);
info->set_filename("/clone-source");
info->set_parentid(0);
info->set_filetype(curve::mds::FileType::INODE_PAGEFILE);
info->set_chunksize(16 * 1024 * 1024);
info->set_length(16 * 1024 * 1024 * 1024UL);
info->set_segmentsize(1 * 1024 * 1024 * 1024ul);

getFileInfoResponse->set_statuscode(curve::mds::StatusCode::kOK);

auto* fakeReturn = new FakeReturn(nullptr, getFileInfoResponse);
responses_.emplace_back(getFileInfoResponse, fakeReturn);

service_.SetGetFileInfoFakeReturn(fakeReturn);
}

protected:
FileServiceOption opt_;
FakeMDSCurveFSService service_;
brpc::Server server_;
const char* kServerAddr = "127.0.0.1:9611";
std::shared_ptr<MDSClient> mdsclient_;
std::vector<std::pair<std::unique_ptr<google::protobuf::Message>,
std::unique_ptr<FakeReturn>>>
responses_;
};

TEST_F(SourceReaderTest, ConcurrentTest) {
SourceReader::GetInstance().SetOption(opt_);
UserInfo user{"user", "pass"};

SourceReader::ReadHandler* handler1 = nullptr;
SourceReader::ReadHandler* handler2 = nullptr;

std::thread th1([&]() {
handler1 = SourceReader::GetInstance().GetReadHandler("/file1", user,
mdsclient_.get());
});
std::thread th2([&]() {
handler2 = SourceReader::GetInstance().GetReadHandler("/file1", user,
mdsclient_.get());
});

th1.join();
th2.join();

ASSERT_EQ(handler1, handler2);
}

} // namespace client
} // namespace curve

0 comments on commit 6d39feb

Please sign in to comment.