Comments (4)
Hi,
Sorry for the delay. Yes it does look like the call to drop should be removed. But perhaps the bigger issue is that we panic in the error handling. Looks like we didn't expect returning the loaned message to ever fail. I am not familiar with the loan message api. What would be an appropriate course of action if returning the loan fails? Try to return again until successful or just leave it be?
from r2r.
That is a good question. I think a panic is not the worst reaction here, there is really no conceivable reason why returning a message should ever fail unless there is some form of corruption. Take a quick look at the errors specified by rcl:
RCL_RET_INVALID_ARGUMENT
,RCL_RET_SUBSCRIPTION_INVALID
are both about passing an invalid parameter. That can't happen unless there is a bug in r2r, so a panic seems the right choice there.RCL_RET_UNSUPPORTED
would be a strange error indeed, as we should have already gotten this when loaning the message. So I'd argue this is also a bug in r2r if it happens, and panicking is the right call
In our case, the way we triggered it is to have some sort of memory corruption on the iceoryx level. So a panic was absolutely appropriate there as well.
So overall, I'd recommend to keep it a panic and touch it again if somebody complains and points out a valid scenario where returning a loaned message can legitimately fail.
edit: by the way, if that argument makes sense to you I'm happy to submit a PR with my proposed approach.
from r2r.
I agree with your comments. I realize that I mistakenly pushed some code I added to test out your scenario. Happy to accept a PR in any case.
from r2r.
Fixed by #97.
from r2r.
Related Issues (20)
- v0.9.0 not compiling on rustc 1.73.0 (cc66ad468 2023-10-03), private type publishers::Publisher_ leaks. HOT 1
- Compile only necessary packages to speed up compilation HOT 5
- Failed docs.rs build HOT 3
- Support for deadline on subscribers HOT 2
- Bug: Incorrect message name conversion to snake case HOT 4
- Implementing Actions HOT 4
- [Discussion] support ROS_TIME from /clock source HOT 1
- Regression: Commit e33a57c causes "'rcl/rcl.h' file not found" HOT 9
- Handling raw messages with `rcl_take_serialized_message` HOT 1
- Public `from_serialized_bytes` and Untyped support HOT 4
- Get publisher infos by topic HOT 2
- How to create a new r2r::builtin_interfaces::msg::Time to represent the current timestamp? I can't seem to find fn HOT 2
- Occasional build failures HOT 2
- Version 0.8.3 won't build, Error E0446 HOT 2
- BUG: Cannot compile library HOT 2
- sensor_msgs cannot found HOT 2
- Question about the `spin_once` function HOT 5
- unsafe precondition(s) violated: ptr::copy_nonoverlapping with rust 1.78 HOT 1
- ROS2 Jazzy support HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from r2r.