Skip to content

Commit 52e59af

Browse files
committed
fix: concurrency Lazy::Get should get correct result
1 parent 4c6f46d commit 52e59af

4 files changed

Lines changed: 76 additions & 10 deletions

File tree

src/iceberg/test/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ add_iceberg_test(util_test
116116
decimal_test.cc
117117
endian_test.cc
118118
formatter_test.cc
119+
lazy_test.cc
119120
location_util_test.cc
120121
roaring_position_bitmap_test.cc
121122
position_delete_index_test.cc

src/iceberg/test/lazy_test.cc

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/util/lazy.h"
21+
22+
#include <gmock/gmock.h>
23+
#include <gtest/gtest.h>
24+
25+
#include "iceberg/result.h"
26+
#include "iceberg/test/matchers.h"
27+
28+
namespace {
29+
30+
struct NonDefaultConstructible {
31+
explicit NonDefaultConstructible(int value) : value(value) {}
32+
NonDefaultConstructible() = delete;
33+
34+
int value;
35+
};
36+
37+
iceberg::Result<NonDefaultConstructible> InitNonDefaultConstructible(int value) {
38+
return NonDefaultConstructible(value);
39+
}
40+
41+
iceberg::Result<int> InitAlwaysFails() { return iceberg::Invalid("init failed"); }
42+
43+
} // namespace
44+
45+
TEST(LazyTest, SupportsNonDefaultConstructibleValues) {
46+
const iceberg::Lazy<InitNonDefaultConstructible> lazy;
47+
48+
auto first = lazy.Get(42);
49+
ASSERT_THAT(first, iceberg::IsOk());
50+
EXPECT_EQ(first->get().value, 42);
51+
52+
auto second = lazy.Get(13);
53+
ASSERT_THAT(second, iceberg::IsOk());
54+
EXPECT_EQ(second->get().value, 42);
55+
EXPECT_EQ(&first->get(), &second->get());
56+
}
57+
58+
TEST(LazyTest, ReusesInitializationError) {
59+
const iceberg::Lazy<InitAlwaysFails> lazy;
60+
61+
auto first = lazy.Get();
62+
EXPECT_THAT(first, iceberg::IsError(iceberg::ErrorKind::kInvalid));
63+
EXPECT_THAT(first, iceberg::HasErrorMessage("init failed"));
64+
65+
auto second = lazy.Get();
66+
EXPECT_THAT(second, iceberg::IsError(iceberg::ErrorKind::kInvalid));
67+
EXPECT_THAT(second, iceberg::HasErrorMessage("init failed"));
68+
}

src/iceberg/test/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ iceberg_tests = {
8989
'decimal_test.cc',
9090
'endian_test.cc',
9191
'formatter_test.cc',
92+
'lazy_test.cc',
9293
'location_util_test.cc',
9394
'position_delete_index_test.cc',
9495
'retry_util_test.cc',

src/iceberg/util/lazy.h

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <concepts>
2626
#include <functional>
2727
#include <mutex>
28+
#include <utility>
2829

2930
#include "iceberg/result.h"
3031
#include "iceberg/util/macros.h"
@@ -48,19 +49,14 @@ class Lazy {
4849
requires std::invocable<decltype(InitFunc), Args...> &&
4950
std::same_as<std::invoke_result_t<decltype(InitFunc), Args...>, Result<T>>
5051
Result<std::reference_wrapper<T>> Get(Args&&... args) const {
51-
Result<T> result;
52-
std::call_once(flag_, [&result, this, &args...]() {
53-
result = InitFunc(std::forward<Args>(args)...);
54-
if (result) {
55-
this->value_ = std::move(result.value());
56-
}
57-
});
58-
ICEBERG_RETURN_UNEXPECTED(result);
59-
return std::ref(value_);
52+
std::call_once(
53+
flag_, [this, &args...]() { value_ = InitFunc(std::forward<Args>(args)...); });
54+
ICEBERG_RETURN_UNEXPECTED(value_);
55+
return std::ref(*value_);
6056
}
6157

6258
private:
63-
mutable T value_;
59+
mutable Result<T> value_ = Invalid("Lazy value has not been initialized");
6460
mutable std::once_flag flag_;
6561
};
6662

0 commit comments

Comments
 (0)