Skip to content

Commit 773b304

Browse files
committed
src: copy-construct data when cloning threadsafe COW
Initialize the cloned data directly while holding the source read lock, so payloads do not need to be default-constructible or copy-assignable. Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
1 parent c8e2a82 commit 773b304

2 files changed

Lines changed: 22 additions & 4 deletions

File tree

src/node_threadsafe_cow-inl.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ T* ThreadsafeCopyOnWrite<T>::Write::operator->() {
4242
}
4343

4444
template <typename T>
45-
ThreadsafeCopyOnWrite<T>::Impl::Impl(const Impl& other) {
46-
RwLock::ScopedReadLock lock(other.mutex);
47-
data = other.data;
48-
}
45+
ThreadsafeCopyOnWrite<T>::Impl::Impl(const Impl& other)
46+
: data([&other]() {
47+
RwLock::ScopedReadLock lock(other.mutex);
48+
return other.data;
49+
}()) {}
4950

5051
} // namespace node
5152

test/cctest/test_per_process.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <string>
88

9+
using node::ThreadsafeCopyOnWrite;
910
using node::builtins::BuiltinLoader;
1011
using node::builtins::BuiltinSourceMap;
1112

@@ -18,6 +19,22 @@ class PerProcessTest : public ::testing::Test {
1819

1920
namespace {
2021

22+
struct CopyConstructOnly {
23+
explicit CopyConstructOnly(int value) : value(value) {}
24+
CopyConstructOnly() = delete;
25+
CopyConstructOnly(const CopyConstructOnly&) = default;
26+
CopyConstructOnly& operator=(const CopyConstructOnly&) = delete;
27+
28+
int value;
29+
};
30+
31+
TEST(ThreadsafeCopyOnWriteTest, CloneCopyConstructsData) {
32+
const ThreadsafeCopyOnWrite<CopyConstructOnly> original(42);
33+
auto copy = original;
34+
35+
EXPECT_EQ(copy.write()->value, 42);
36+
}
37+
2138
TEST_F(PerProcessTest, EmbeddedSources) {
2239
const auto& sources = PerProcessTest::get_sources_for_test();
2340
ASSERT_TRUE(std::any_of(sources.cbegin(), sources.cend(), [](auto p) {

0 commit comments

Comments
 (0)