Skip to content

Commit fcfb27c

Browse files
committed
hmon: rework error handling in FFI
- Uniformly return `FFICode` in FFI functions. - Unit tests for FFI functions in Rust. - Rework comments for FFI functions. - Move `crate::common::ffi` to `crate::ffi`.
1 parent 997ffa1 commit fcfb27c

13 files changed

Lines changed: 1433 additions & 402 deletions

File tree

src/health_monitoring_lib/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ PROC_MACRO_DEPS = [
2828
CC_SOURCES = [
2929
"cpp/common.cpp",
3030
"cpp/deadline_monitor.cpp",
31-
"cpp/ffi_helpers.h",
3231
"cpp/health_monitor.cpp",
3332
]
3433

src/health_monitoring_lib/cpp/common.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,19 @@
1111
* SPDX-License-Identifier: Apache-2.0
1212
********************************************************************************/
1313
#include <score/hm/common.h>
14+
#include <cstdlib>
1415

1516
namespace score::hm::internal
1617
{
1718

19+
void abort_on_error(FFICode code)
20+
{
21+
if (code != kSuccess)
22+
{
23+
std::abort();
24+
}
25+
}
26+
1827
DroppableFFIHandle::DroppableFFIHandle(FFIHandle handle, DropFn drop_fn) : handle_(handle), drop_fn_(drop_fn) {}
1928

2029
DroppableFFIHandle::DroppableFFIHandle(DroppableFFIHandle&& other) noexcept

src/health_monitoring_lib/cpp/deadline_monitor.cpp

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,47 @@
1111
* SPDX-License-Identifier: Apache-2.0
1212
********************************************************************************/
1313
#include "score/hm/deadline/deadline_monitor.h"
14-
#include "ffi_helpers.h"
1514

15+
namespace
16+
{
1617
extern "C" {
1718
using namespace score::hm;
1819
using namespace score::hm::internal;
1920
using namespace score::hm::deadline;
2021

21-
// Deadline monitoring Foreign Function Interface Declarations that are exported by Rust implementation library
22-
internal::FFIHandle deadline_monitor_builder_create();
23-
void deadline_monitor_builder_destroy(internal::FFIHandle handle);
24-
void deadline_monitor_builder_add_deadline(internal::FFIHandle handler,
25-
const IdentTag* tag,
26-
uint32_t min,
27-
uint32_t max);
28-
int deadline_monitor_cpp_get_deadline(FFIHandle handler, const IdentTag* tag, FFIHandle* out);
29-
void deadline_monitor_cpp_destroy(FFIHandle handler);
30-
void deadline_destroy(FFIHandle deadline_handle);
31-
int deadline_start(FFIHandle deadline_handle);
32-
void deadline_stop(FFIHandle deadline_handle);
22+
// Functions below must match functions defined in `crate::deadline::ffi`.
23+
24+
FFICode deadline_monitor_builder_create(FFIHandle* deadline_monitor_builder_handle_out);
25+
FFICode deadline_monitor_builder_destroy(FFIHandle deadline_monitor_builder_handle);
26+
FFICode deadline_monitor_builder_add_deadline(FFIHandle deadline_monitor_builder_handle,
27+
const IdentTag* deadline_tag,
28+
uint32_t min_ms,
29+
uint32_t max_ms);
30+
FFICode deadline_monitor_get_deadline(FFIHandle deadline_monitor_handle,
31+
const IdentTag* deadline_tag,
32+
FFIHandle* deadline_handle_out);
33+
FFICode deadline_monitor_destroy(FFIHandle deadline_monitor_handle);
34+
FFICode deadline_destroy(FFIHandle deadline_handle);
35+
FFICode deadline_start(FFIHandle deadline_handle);
36+
FFICode deadline_stop(FFIHandle deadline_handle);
37+
}
38+
39+
FFIHandle deadline_monitor_builder_create_wrapper()
40+
{
41+
FFIHandle handle{nullptr};
42+
auto result{deadline_monitor_builder_create(&handle)};
43+
abort_on_error(result);
44+
return handle;
3345
}
3446

47+
} // namespace
48+
3549
// C++ wrapper for Rust library - the API implementation obeys the Rust API semantics and it's invariants
3650

3751
namespace score::hm::deadline
3852
{
3953
DeadlineMonitorBuilder::DeadlineMonitorBuilder()
40-
: monitor_builder_handler_(deadline_monitor_builder_create(), &deadline_monitor_builder_destroy)
54+
: monitor_builder_handler_{deadline_monitor_builder_create_wrapper(), &deadline_monitor_builder_destroy}
4155
{
4256
}
4357

@@ -46,30 +60,30 @@ DeadlineMonitorBuilder DeadlineMonitorBuilder::add_deadline(const IdentTag& tag,
4660
auto handle = monitor_builder_handler_.as_rust_handle();
4761
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
4862

49-
deadline_monitor_builder_add_deadline(handle.value(), &tag, range.min_ms(), range.max_ms());
63+
auto result{deadline_monitor_builder_add_deadline(handle.value(), &tag, range.min_ms(), range.max_ms())};
64+
abort_on_error(result);
5065

5166
return std::move(*this);
5267
}
5368

54-
DeadlineMonitor::DeadlineMonitor(FFIHandle handle) : monitor_handle_(handle, &deadline_monitor_cpp_destroy) {}
69+
DeadlineMonitor::DeadlineMonitor(FFIHandle handle) : monitor_handle_(handle, &deadline_monitor_destroy) {}
5570

5671
score::cpp::expected<Deadline, score::hm::Error> DeadlineMonitor::get_deadline(const IdentTag& tag)
5772
{
5873
auto handle = monitor_handle_.as_rust_handle();
5974
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
6075

61-
internal::FFIHandle ret = nullptr;
62-
auto result = deadline_monitor_cpp_get_deadline(handle.value(), &tag, &ret);
63-
76+
FFIHandle ret = nullptr;
77+
auto result = deadline_monitor_get_deadline(handle.value(), &tag, &ret);
6478
if (result != kSuccess)
6579
{
66-
return score::cpp::unexpected(::score::hm::ffi::fromRustError(result));
80+
return score::cpp::unexpected(static_cast<Error>(result));
6781
}
6882

6983
return score::cpp::expected<Deadline, score::hm::Error>(Deadline{ret});
7084
}
7185

72-
Deadline::Deadline(internal::FFIHandle handle) : deadline_handle_(handle, &deadline_destroy), has_handle_(false) {}
86+
Deadline::Deadline(FFIHandle handle) : deadline_handle_(handle, &deadline_destroy), has_handle_(false) {}
7387

7488
Deadline::~Deadline()
7589
{
@@ -90,7 +104,7 @@ score::cpp::expected<DeadlineHandle, score::hm::Error> Deadline::start()
90104
auto result = deadline_start(handle.value());
91105
if (result != kSuccess)
92106
{
93-
return score::cpp::unexpected(::score::hm::ffi::fromRustError(result));
107+
return score::cpp::unexpected(static_cast<Error>(result));
94108
}
95109

96110
has_handle_ = true;
@@ -109,7 +123,8 @@ void DeadlineHandle::stop()
109123
auto handle = deadline_.value().get().deadline_handle_.as_rust_handle();
110124
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
111125

112-
deadline_stop(handle.value());
126+
auto result{deadline_stop(handle.value())};
127+
abort_on_error(result);
113128
}
114129
}
115130

src/health_monitoring_lib/cpp/ffi_helpers.h

Lines changed: 0 additions & 43 deletions
This file was deleted.

src/health_monitoring_lib/cpp/health_monitor.cpp

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,44 +12,62 @@
1212
********************************************************************************/
1313
#include "score/hm/health_monitor.h"
1414

15+
namespace
16+
{
1517
extern "C" {
1618
using namespace score::hm;
19+
using namespace score::hm::internal;
20+
using namespace score::hm::deadline;
21+
22+
// Functions below must match functions defined in `crate::ffi`.
23+
24+
FFICode health_monitor_builder_create(FFIHandle* health_monitor_builder_handle_out);
25+
FFICode health_monitor_builder_destroy(FFIHandle health_monitor_builder_handle);
26+
FFICode health_monitor_builder_build(FFIHandle health_monitor_builder_handle,
27+
uint32_t supervisor_cycle_ms,
28+
uint32_t internal_cycle_ms,
29+
FFIHandle* health_monitor_handle_out);
30+
FFICode health_monitor_builder_add_deadline_monitor(FFIHandle health_monitor_builder_handle,
31+
const IdentTag* monitor_tag,
32+
FFIHandle deadline_monitor_builder_handle);
33+
FFICode health_monitor_get_deadline_monitor(FFIHandle health_monitor_handle,
34+
const IdentTag* monitor_tag,
35+
FFIHandle* deadline_monitor_handle_out);
36+
FFICode health_monitor_start(FFIHandle health_monitor_handle);
37+
FFICode health_monitor_destroy(FFIHandle health_monitor_handle);
38+
}
1739

18-
// Health Monitor Foreign Function Interface Declarations that are exported by Rust implementation library
19-
internal::FFIHandle health_monitor_builder_create();
20-
void health_monitor_builder_destroy(internal::FFIHandle handler);
21-
22-
internal::FFIHandle health_monitor_builder_build(internal::FFIHandle health_monitor_builder_handle,
23-
uint32_t supervisor_cycle_ms,
24-
uint32_t internal_cycle_ms);
25-
void health_monitor_builder_add_deadline_monitor(internal::FFIHandle handle,
26-
const IdentTag* tag,
27-
internal::FFIHandle monitor_handle);
28-
29-
internal::FFIHandle health_monitor_get_deadline_monitor(internal::FFIHandle health_monitor_handle, const IdentTag* tag);
30-
void health_monitor_start(internal::FFIHandle health_monitor_handle);
31-
void health_monitor_destroy(internal::FFIHandle handler);
40+
FFIHandle health_monitor_builder_create_wrapper()
41+
{
42+
FFIHandle handle{nullptr};
43+
auto result{health_monitor_builder_create(&handle)};
44+
abort_on_error(result);
45+
return handle;
3246
}
3347

48+
} // namespace
49+
3450
// C++ wrapper for Rust library - the API implementation obeys the Rust API semantics and it's invariants
3551

3652
namespace score::hm
3753
{
3854

3955
HealthMonitorBuilder::HealthMonitorBuilder()
40-
: health_monitor_builder_handle_{health_monitor_builder_create(), &health_monitor_builder_destroy}
56+
: health_monitor_builder_handle_{health_monitor_builder_create_wrapper(), &health_monitor_builder_destroy}
4157
{
4258
}
4359

4460
HealthMonitorBuilder HealthMonitorBuilder::add_deadline_monitor(const IdentTag& tag,
45-
deadline::DeadlineMonitorBuilder&& monitor) &&
61+
DeadlineMonitorBuilder&& monitor) &&
4662
{
4763
auto monitor_handle = monitor.drop_by_rust();
4864
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(monitor_handle.has_value());
4965
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle_.as_rust_handle().has_value());
5066

51-
health_monitor_builder_add_deadline_monitor(
52-
health_monitor_builder_handle_.as_rust_handle().value(), &tag, monitor_handle.value());
67+
auto result{health_monitor_builder_add_deadline_monitor(
68+
health_monitor_builder_handle_.as_rust_handle().value(), &tag, monitor_handle.value())};
69+
abort_on_error(result);
70+
5371
return std::move(*this);
5472
}
5573

@@ -67,16 +85,21 @@ HealthMonitorBuilder HealthMonitorBuilder::with_supervisor_api_cycle(std::chrono
6785

6886
HealthMonitor HealthMonitorBuilder::build() &&
6987
{
70-
auto handle = health_monitor_builder_handle_.drop_by_rust();
71-
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
88+
auto health_monitor_builder_handle = health_monitor_builder_handle_.drop_by_rust();
89+
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle.has_value());
7290

7391
uint32_t supervisor_duration_ms = static_cast<uint32_t>(supervisor_api_cycle_duration_.count());
7492
uint32_t internal_duration_ms = static_cast<uint32_t>(internal_processing_cycle_duration_.count());
7593

76-
return HealthMonitor(health_monitor_builder_build(handle.value(), supervisor_duration_ms, internal_duration_ms));
94+
FFIHandle health_monitor_handle{nullptr};
95+
auto result{health_monitor_builder_build(
96+
health_monitor_builder_handle.value(), supervisor_duration_ms, internal_duration_ms, &health_monitor_handle)};
97+
abort_on_error(result);
98+
99+
return HealthMonitor{health_monitor_handle};
77100
}
78101

79-
HealthMonitor::HealthMonitor(internal::FFIHandle handle) : health_monitor_(handle)
102+
HealthMonitor::HealthMonitor(FFIHandle handle) : health_monitor_(handle)
80103
{
81104
// Initialize health monitor
82105
}
@@ -87,21 +110,22 @@ HealthMonitor::HealthMonitor(HealthMonitor&& other)
87110
other.health_monitor_ = nullptr;
88111
}
89112

90-
score::cpp::expected<deadline::DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
113+
score::cpp::expected<DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
91114
{
92-
auto maybe_monitor = health_monitor_get_deadline_monitor(health_monitor_, &tag);
93-
94-
if (maybe_monitor != nullptr)
115+
FFIHandle handle{nullptr};
116+
auto result{health_monitor_get_deadline_monitor(health_monitor_, &tag, &handle)};
117+
if (result != kSuccess)
95118
{
96-
97-
return score::cpp::expected<deadline::DeadlineMonitor, Error>(deadline::DeadlineMonitor{maybe_monitor});
119+
return score::cpp::unexpected(static_cast<Error>(result));
98120
}
99121

100-
return score::cpp::unexpected(Error::NotFound);
122+
return score::cpp::expected<DeadlineMonitor, Error>(DeadlineMonitor{handle});
101123
}
124+
102125
void HealthMonitor::start()
103126
{
104-
health_monitor_start(health_monitor_);
127+
auto result{health_monitor_start(health_monitor_)};
128+
abort_on_error(result);
105129
}
106130

107131
HealthMonitor::~HealthMonitor()

0 commit comments

Comments
 (0)