Skip to content

Commit

Permalink
fix: possible exception in request_context (#5784)
Browse files Browse the repository at this point in the history
Description
---
Fixed possible exception in `pub struct RequestContext` that could occur
when splitting the request and reply channel in `pub fn split`

Motivation and Context
---
The code should not be allowed to panic in use in the wrong context.

How Has This Been Tested?
---
Unit tests

What process can a PR reviewer use to test or verify this change?
---
Code walk through

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
  • Loading branch information
hansieodendaal and SWvheerden committed Sep 22, 2023
1 parent 467a8d4 commit 6c8e2d3
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 18 deletions.
2 changes: 1 addition & 1 deletion base_layer/service_framework/src/lib.rs
Expand Up @@ -52,7 +52,7 @@
//! // At the same time receive the request and reply
//! async move {
//! let req_context = receiver.next().await.unwrap();
//! let msg = req_context.request().unwrap().clone();
//! let msg = req_context.request().clone();
//! req_context.reply(msg.to_uppercase());
//! }
//! );
Expand Down
22 changes: 5 additions & 17 deletions base_layer/service_framework/src/reply_channel.rs
Expand Up @@ -131,37 +131,25 @@ impl<T> Future for TransportResponseFuture<T> {
/// request.
pub struct RequestContext<TReq, TResp> {
reply_tx: oneshot::Sender<TResp>,
request: Option<TReq>,
request: TReq,
}

impl<TReq, TResp> RequestContext<TReq, TResp> {
/// Create a new RequestContect
pub fn new(request: TReq, reply_tx: oneshot::Sender<TResp>) -> Self {
Self {
request: Some(request),
reply_tx,
}
Self { request, reply_tx }
}

/// Return a reference to the request object. None is returned after take_request has
/// been called.
pub fn request(&self) -> Option<&TReq> {
self.request.as_ref()
}

/// Take ownership of the request object, if ownership has not already been taken,
/// otherwise None is returned.
pub fn take_request(&mut self) -> Option<TReq> {
self.request.take()
pub fn request(&self) -> &TReq {
&self.request
}

/// Consume this object and return it's parts. Namely, the request object and
/// the reply oneshot channel.
pub fn split(self) -> (TReq, oneshot::Sender<TResp>) {
(
self.request.expect("RequestContext must be initialized with a request"),
self.reply_tx,
)
(self.request, self.reply_tx)
}

/// Sends a reply to the caller
Expand Down

0 comments on commit 6c8e2d3

Please sign in to comment.