common: Fix deadlock in channel pressure handling

[martinpitt]

When a channel applies back-pressure to its input because its output
queue is getting too large, it relies on the consumer to acknowledge the
reads by ponging to the pings that the channel sent out.

However, it can happen that the last ping was sent for a sequence number
before the one that the channel expects for lifting the pressure. Then
the receiver could drain the output queue and dutifully respond to all
pings, but the last sequence number would still be smaller than the
treshold for lifting the pressure. But the channel isn't sending
anything else either and thus never generates new pings.

Fix this by sending an additional ping when going into pressure mode, to
ensure that the last pong sequence number is within the out_window
again.

This can also be reproduced/investigated with this test-spawn-proc unit
test:

QUnit.only("stream large output - find", function (assert) {
    const done = assert.async();
    assert.expect(2);

    cockpit.spawn(["find", "/"], { err: "ignore" })
            .stream(function(resp) {
                console.log("got block", resp.length);
            })
            .then(function(resp) {
                assert.equal(resp, "", "no done data");
            })
            .always(function() {
                assert.equal(this.state(), "resolved", "didn't fail");
                done();
            });
});

But this is not appropriate for actually committing -- the find often
fails at the end, and takes too many IO resources for a unit test. The
integration test confirms the fix, though.

Fixes #12580
https://bugzilla.redhat.com/show_bug.cgi?id=1751783

• Builds on top of various unit test fixes: PR Various unit test/debugging fixes #14075
• Update PR reference in integration test
• Revert the order again and beautify the code

[martinpitt]

I talked this over with @stefwalter , and he confirmed that this is a valid fix. The actual new flooding test in check-terminal succeeds everywhere, I just botched the subsequent checks. I can't run this locally due to chromium changes. I have a fix prepared locally, but I also still want to reformat the code a bit.

[martinpitt]

PR #14075 landed, and I applied the code clenaup. @stefwalter, do you want to officially review that?

[mvollmer]

Nice! This makes sense to me, for what it's worth.

[martinpitt]

@mvollmer brought up that it's not clear that we avoid ping/pong loops with the same sequence number. I changed the level to edge triggering to ensure this.

[stefwalter]

If this is edge triggered then the variable name needs to change. The variable is named for a level.

[martinpitt]

Fair enough. I renamed it to trigger_pressure.

[stefwalter]

That means this emit_pressure call here has also changed from level triggered to edge triggered. Is that in line with all the other uses of this function call? Worth doing a check.

[stefwalter]

src/bridge/cockpitpacketchannel.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);
src/bridge/cockpitpacketchannel.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

Edge triggered. ^^

src/bridge/cockpitstream.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);
src/bridge/cockpitstream.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

Edge triggered ^^

src/common/cockpitchannel.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);

This other call in CockpitChannel is edge triggered. But may have an off by one on line 248. I need to double check this.

src/common/cockpitchannel.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

This is the one we're changing.

src/common/cockpitpipe.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);
src/common/cockpitpipe.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

Edge triggered ^^

src/common/cockpitwebresponse.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);
src/common/cockpitwebresponse.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

Edge triggered ^^

src/websocket/websocketconnection.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), FALSE);
src/websocket/websocketconnection.c: cockpit_flow_emit_pressure (COCKPIT_FLOW (self), TRUE);

Edge triggered ^^

[martinpitt]

As we just figured out, all other pressure calls use edge triggering already, so that gets cockpitchannel.c in line with the others.