fix: Encoder state machine tolerates being wrapped by an AsyncWrite

[aatifsyed]

Simplified the Encoder state machine.
It should now handle poll_flush calls after poll_shutdown calls.

Closes #308
See also #246

[NobodyXu]

Thanks!

You are right, poll_shutdown should indeed call self.poll_flush before calling inner.poll_shutdown

[NobodyXu]

Thanks!

You are right, poll_shutdown should indeed call self.poll_flush before calling inner.poll_shutdown

[NobodyXu]

Thanks, mostly LGTM

cc @robjtede can you take a look at this please?

[NobodyXu]

I think we should test poll_shutdown by keeping track of numbers of calls to underlying poll_flush?

[aatifsyed]

I think that feels a bit excessive - we know that poll_shutdown has been called twice because of how DelayedShutdown is implemented (only returns Poll::Ready the second time), and that's what we really want to test.

i.e I don't think a change like this actually does anything for the test:

-    let mut zstd_encoder = Wrapper::new(Trace::new(ZstdEncoder::new(DelayedShutdown::default())));
+    let mut delayed_shutdown = DelayedShutdown::default();
+    let mut zstd_encoder = Wrapper::new(Trace::new(ZstdEncoder::new(&mut delayed_shutdown)));
     futures::executor::block_on(zstd_encoder.shutdown()).unwrap();
+    assert_eq!(delayed_shutdown.num_times_shutdown_called, 1);

[NobodyXu]

I'd want the poll_flush to be tested, by making sure it is called if and only if poll_shutdown is called.

[aatifsyed]

:) could you help me write this test?

[NobodyXu]

Bit busy now, but in general, I think we want something like this:

struct DummyWriter {
    is_poll_flush_called: boolean,
    is_poll_shutdown_called: usize,
}

impl AsyncWrite {
    fn poll_flush(...) {
        assert!(!self.is_poll_shutdown_called);
        self.is_poll_flush_called = true;
        Ready(Ok())
    }

    fn poll_shutdown(...) {
        self.is_poll_shutdown_called = true;
        Ready(Ok())
    }
}

And then after shutdown is called, we check that both is set to true to verify this fix.