Do not trigger transport error in case of SHM buffer invalidation #1245
Conversation
| @@ -330,12 +331,17 @@ pub fn map_zslice_to_shmbuf(zslice: &mut ZSlice, shmr: &ShmReader) -> ZResult<() | |||
| // Deserialize the shminfo | |||
| let shmbinfo: ShmBufInfo = codec.read(&mut reader).map_err(|e| zerror!("{:?}", e))?; | |||
|
|
|||
| // Mount shmbuf | |||
| let smb = shmr.read_shmbuf(&shmbinfo)?; | |||
There was a problem hiding this comment.
I'm not sure about this solution. If we do that the only result we get is that the shm_buf is remapped and propagated anyway in the system. As a result, the user will eventually receive an empty message.
A better approach would be to let the caller of this function to properly handle the failure case.
In practical terms, in case of failure we don't call the routing callback and just drop this message without closing the link. Opinions?
There was a problem hiding this comment.
I thought about dropping the whole message, but the problem is that ZBuf might contain many ZSlice's and not necessary all of them will be invalidated - what should we do in this case? Drop the whole ZBuf and potentially loose a lot of data? - I think this is not a good option.
There was a problem hiding this comment.
Such "punctured" ZBuf can also be a bad thing because it can be completely broken from user's perspective, but it is way better to give user decide by exposing this buffer out instead of implicitly dropping it
There was a problem hiding this comment.
My point is that we may be a router and propagate an already invalid message across different hops to eventually reach the user application. Having a punctured ZBuf is not a good option either because we don't explicitly signal that to the user and we are not safe from the case where a punctured ZBuf is a valid ZBuf, leading to the case where we are replacing user's content that becomes valid by chance.
Because of the above I believe it is still safer and clearer from a behavioural point of view (principle of least surprise) to drop the message and potentially log it.
There was a problem hiding this comment.
Okay, I will replace this behavior!
The core problem is that our RX task goes down with an error if it receives invalidated SHM buffer. And SHM buffer invalidation is not a hard error, but a case that could happen, so we shouldn't fail our transport because of it