microsoft / yardl

Tooling for streaming instrument data
https://microsoft.github.io/yardl/
MIT License
29 stars 5 forks source link

Error reading empty stream if it is the last step in a Protocol #123

Closed naegelejd closed 5 months ago

naegelejd commented 5 months ago

If:

  1. the last step in a Protocol is a stream, and
  2. the user is batch reading the stream, and
  3. the stream is empty, the Reader.Close() method throws an incorrect error.

Using 28aa4af and the following model:

EmptyTest: !protocol
  sequence:
    strings: !stream
      items: string

and the following demonstration program:

int main(void) {
  ::binary::EmptyTestWriter w("test.bin");

  std::vector<std::string> strings;
  w.WriteStrings(strings);
  w.EndStrings();
  w.Close();

  ::binary::EmptyTestReader r("test.bin");

  int count = 0;
  strings.reserve(10);
  while (r.ReadStrings(strings)) {
    for (auto const& s : strings) {
      (void)(s);
      count++;
    }
  }
  assert(count == 0);

  r.Close();

  return 0;
}

the call to r.Close() throws the following error:

terminate called after throwing an instance of 'std::runtime_error'
  what():  Expected call to ReadStrings() but received call to Close() instead.
naegelejd commented 5 months ago

This bug can be avoided (masked) by

  1. adding another step after the stream and reading/writing it accordingly, or
  2. using the "singular" r.ReadStrings(std::string) read method

The reason is that the generated Close method doesn't perform the same state check(s) that are performed per-step, specifically the

if stepIndex > 0 && protocol.Sequence[stepIndex-1].IsStream() {
    fmt.Fprintf(w, "if (state_ == %d) {\n", previousUnobservedcompletionState)
    ...

check currently performed in the writeReaderStateCheckIfStatement function in protocols.go.

Solution: We can just add this check for stream steps in the generated Close method.