esa / CCSDS_MO_TRANS

REPOSITORY ARCHIVED - for the latest version please go to https://github.com/esa/mo-services-java
Other
3 stars 7 forks source link

Synchronously decode message from InputStream #10

Open gbonnefille opened 7 years ago

gbonnefille commented 7 years ago

When a transport uses an InputStream (ie a Socket) we have to ensure a full decoding is done before trying to read a new/next message. If not, we can fall into a case were message is not yet fully decoded, parts of it are still in the InputStream while the transport check the InputStream for new bytes.

Currently, data availability is done by GENMessageReceiver<InputStream>.readEncodedMessage while decoding is done by GENIncomingMessageDecoder.decodeAndCreateMessage() run by GENIncomingMessageReceiver.run() executed by a dedicated ExecutorService.

What is the design to allow a more synchronous processing?

SamCooper commented 7 years ago

The encoding needs to encode somewhere on the message header the full length of the encoded message so that it can be extracted from the stream at the read stage rather than the decode stage.

Currently the transports/encodings have not done this as they have not needed to be performant. I think the latest TCP/IP specification does do this so I would expect better behaviour on this point with that.

gbonnefille commented 7 years ago

In my understanding, the current design expects the read stage is able to pick all related bytes from the stream before doing any decoding operation.

With the latest TCP/IP transport specification, we have to decode at least the header to know the length of the full message. And a full decoding is necessary because the header contains variable parts not covered by any length information.

SamCooper commented 7 years ago

I thought the TCP/IP spec used the TCP/IP length field to hold the packet length?

gbonnefille commented 7 years ago

As far as I know, TCP/IP is stream oriented and thus, at application level, we do not have access to any packet length information.

At the same time, reading the standard again, I'm not sure to understand what the body length is supposed to cover: only body or all following bytes (optional parts and body).

SamCooper commented 7 years ago

Talk to Dario, its his specification

dmarszk commented 5 years ago

@gbonnefille Can you confirm that the issue is fixed? Albeit rather dirty, I think the attached code works. Or do you thing that the call should be made synchronous in addition? https://github.com/esa/CCSDS_MO_TRANS/blob/master/CCSDS_MAL_TRANSPORT_TCPIP/src/main/java/esa/mo/mal/transport/tcpip/TCPIPTransportDataTransceiver.java#L108