From 36f319e954d129951ba2450b782f05afaa149b55 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 23 Apr 2025 15:17:28 -0400 Subject: [PATCH 1/3] respect ctx deadline when adding module resources --- src/viam/sdk/module/service.cpp | 5 +++-- src/viam/sdk/rpc/server.cpp | 16 ++++++++++++++++ src/viam/sdk/rpc/server.hpp | 8 ++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index b270f629d..6ac6a51d3 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -54,7 +54,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { ModuleService& parent; // TODO(RSDK-6528) - to the extent possible, switch to using `server_helper` - ::grpc::Status AddResource(::grpc::ServerContext*, + ::grpc::Status AddResource(::grpc::ServerContext* ctx, const ::viam::module::v1::AddResourceRequest* request, ::viam::module::v1::AddResourceResponse*) override { const viam::app::v1::ComponentConfig& proto = request->config(); @@ -74,7 +74,8 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { } }; try { - parent.server_->add_resource(res); + auto deadline = ctx->deadline(); + parent.server_->add_resource(res, &deadline); } catch (const std::exception& exc) { return grpc::Status(::grpc::INTERNAL, exc.what()); } diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index c16394b26..1f23db435 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -44,7 +44,23 @@ void Server::register_service(grpc::Service* service) { builder_->RegisterService(service); } +namespace { +bool deadline_exceeded(const std::chrono::system_clock::time_point& deadline) { + std::cout << "deadline is " << deadline.time_since_epoch().count() << " and now is " + << std::chrono::system_clock::now().time_since_epoch().count() << "\n\n"; + return deadline < std::chrono::system_clock::now(); // deadline is before now +} +} // namespace + void Server::add_resource(std::shared_ptr resource) { + add_resource(std::move(resource), nullptr); +} + +void Server::add_resource(std::shared_ptr resource, + std::chrono::system_clock::time_point* deadline) { + if (deadline && deadline_exceeded(*deadline)) { + throw Exception(ErrorCondition::k_general, "Deadline expired"); + } auto api = resource->api(); if (managed_servers_.find(api) == managed_servers_.end()) { std::ostringstream buffer; diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index e735a6351..bb2ca9bae 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -50,6 +50,14 @@ class Server { /// @throws `Exception` if a matching `ResourceServer` doesn't exist in the server. void add_resource(std::shared_ptr resource); + /// @brief Adds a specific managed resource to the associated resource server + /// @param resource The resource to add + /// @param deadline Deadline after which to not add the resource + /// @throws `Exception` if a matching `ResourceServer` doesn't exist in the server. + /// @throws `Exception` if the deadline is not nil and has passed + void add_resource(std::shared_ptr resource, + std::chrono::system_clock::time_point* deadline); + /// @brief Adds a listening port to the server. /// @param address The address to listen at. /// @param creds The server credentials; defaults to a insecure server credentials. From 7baff37d68b491a1360d16d90c77dda707207e9c Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 23 Apr 2025 15:19:39 -0400 Subject: [PATCH 2/3] remove print statement --- src/viam/sdk/rpc/server.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 1f23db435..62d9c9620 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -46,8 +46,6 @@ void Server::register_service(grpc::Service* service) { namespace { bool deadline_exceeded(const std::chrono::system_clock::time_point& deadline) { - std::cout << "deadline is " << deadline.time_since_epoch().count() << " and now is " - << std::chrono::system_clock::now().time_since_epoch().count() << "\n\n"; return deadline < std::chrono::system_clock::now(); // deadline is before now } } // namespace From 2d7dc4c3cf9c0ebccadbcb4727148e8f84d97e82 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 28 Apr 2025 08:02:29 -0400 Subject: [PATCH 3/3] use boost optional --- src/viam/examples/modules/complex/proto/buf.lock | 4 ++-- src/viam/sdk/module/service.cpp | 3 +-- src/viam/sdk/rpc/server.cpp | 4 ++-- src/viam/sdk/rpc/server.hpp | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/viam/examples/modules/complex/proto/buf.lock b/src/viam/examples/modules/complex/proto/buf.lock index 73ebfcf9b..2ca276594 100644 --- a/src/viam/examples/modules/complex/proto/buf.lock +++ b/src/viam/examples/modules/complex/proto/buf.lock @@ -4,5 +4,5 @@ deps: - remote: buf.build owner: googleapis repository: googleapis - commit: 751cbe31638d43a9bfb6162cd2352e67 - digest: shake256:87f55470d9d124e2d1dedfe0231221f4ed7efbc55bc5268917c678e2d9b9c41573a7f9a557f6d8539044524d9fc5ca8fbb7db05eb81379d168285d76b57eb8a4 + commit: 61b203b9a9164be9a834f58c37be6f62 + digest: shake256:e619113001d6e284ee8a92b1561e5d4ea89a47b28bf0410815cb2fa23914df8be9f1a6a98dcf069f5bc2d829a2cfb1ac614863be45cd4f8a5ad8606c5f200224 diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 6ac6a51d3..738ffae91 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -74,8 +74,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { } }; try { - auto deadline = ctx->deadline(); - parent.server_->add_resource(res, &deadline); + parent.server_->add_resource(res, ctx->deadline()); } catch (const std::exception& exc) { return grpc::Status(::grpc::INTERNAL, exc.what()); } diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 62d9c9620..c78975044 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -51,11 +51,11 @@ bool deadline_exceeded(const std::chrono::system_clock::time_point& deadline) { } // namespace void Server::add_resource(std::shared_ptr resource) { - add_resource(std::move(resource), nullptr); + add_resource(std::move(resource), boost::none); } void Server::add_resource(std::shared_ptr resource, - std::chrono::system_clock::time_point* deadline) { + boost::optional deadline) { if (deadline && deadline_exceeded(*deadline)) { throw Exception(ErrorCondition::k_general, "Deadline expired"); } diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index bb2ca9bae..b938f7b1d 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -56,7 +56,7 @@ class Server { /// @throws `Exception` if a matching `ResourceServer` doesn't exist in the server. /// @throws `Exception` if the deadline is not nil and has passed void add_resource(std::shared_ptr resource, - std::chrono::system_clock::time_point* deadline); + boost::optional deadline); /// @brief Adds a listening port to the server. /// @param address The address to listen at.