sustrik / libdill

Structured concurrency in C
MIT License
1.68k stars 155 forks source link

hdone and hclose #110

Open vlm opened 7 years ago

vlm commented 7 years ago

@sustrik, I am not sure I am following the model correctly.

When I am doing bsend() followed by hclose(), the remote end (such as curl) receives RST and prints out:

curl: (56) Recv failure: Connection reset by peer

When I am doing bsend(); hdone(); hclose(), same error is reported by curl.

If I am not doing hclose() (bsend() followed by hdone() only), I am risking leaking some resources.

Please advise.

Collateral

When hdone() is invoked prior to hclose(), this is what happens from time to time:

Process 59354 stopped
* thread #1, stop reason = signal SIGBUS: hardware error
    frame #0: 0x0000000800bd2246 libdill.so.14`hdone(h=93, deadline=15985994575) at handle.c:154
   151      struct dill_ctx_handle *ctx = &dill_getctx->handle;
   152      CHECKHANDLE(h, -1);
   153      if(dill_slow(!hndl->vfs->done)) {errno = ENOTSUP; return -1;}
-> 154      return hndl->vfs->done(hndl->vfs, deadline);
   155  }
   156
(lldb) 
pudelkoM commented 7 years ago

I could reproduce your issue on Linux:

#include <libdill.h>
#include <assert.h>

int main() {
    struct ipaddr addr;
    int rc = ipaddr_local(&addr, "::1", 9000, 0);
    assert(rc == 0);
    int srv = tcp_listen(&addr, 4);
    while(1) {
        int c;
        c = tcp_accept(srv, NULL, -1);
        if(c == -1) break;
        const char msg[] = "hello, world";
        int rc = bsend(c, msg, sizeof(msg), -1);
        assert(rc != -1);
        hdone(c, -1);
        //msleep(now() + 100);
        hclose(c);
    }
    hclose(srv);
}

With msleep():

01:47:36.954295 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 94: ip6-localhost.35894 > ip6-localhost.9000: Flags [S], seq 1612525249, win 43690, options [mss 65476,sackOK,TS val 9407524 ecr 0,nop,wscale 7], length 0
01:47:36.954327 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 94: ip6-localhost.9000 > ip6-localhost.35894: Flags [S.], seq 1095158369, ack 1612525250, win 43690, options [mss 65476,sackOK,TS val 9407524 ecr 9407524,nop,wscale 7], length 0
01:47:36.954338 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.35894 > ip6-localhost.9000: Flags [.], ack 1, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 0
01:47:36.954604 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 99: ip6-localhost.9000 > ip6-localhost.35894: Flags [P.], seq 1:14, ack 1, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 13
01:47:36.954613 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.35894 > ip6-localhost.9000: Flags [.], ack 14, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 0
01:47:36.954625 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.9000 > ip6-localhost.35894: Flags [F.], seq 14, ack 1, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 0
01:47:36.954659 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.35894 > ip6-localhost.9000: Flags [F.], seq 1, ack 15, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 0
01:47:36.954663 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.9000 > ip6-localhost.35894: Flags [.], ack 2, win 342, options [nop,nop,TS val 9407524 ecr 9407524], length 0

Without msleep():

01:47:06.393856 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 94: ip6-localhost.35892 > ip6-localhost.9000: Flags [S], seq 2077568502, win 43690, options [mss 65476,sackOK,TS val 9399884 ecr 0,nop,wscale 7], length 0
01:47:06.393884 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 94: ip6-localhost.9000 > ip6-localhost.35892: Flags [S.], seq 4294009798, ack 2077568503, win 43690, options [mss 65476,sackOK,TS val 9399884 ecr 9399884,nop,wscale 7], length 0
01:47:06.393894 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.35892 > ip6-localhost.9000: Flags [.], ack 1, win 342, options [nop,nop,TS val 9399884 ecr 9399884], length 0
01:47:06.393980 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 99: ip6-localhost.9000 > ip6-localhost.35892: Flags [P.], seq 1:14, ack 1, win 342, options [nop,nop,TS val 9399884 ecr 9399884], length 13
01:47:06.393988 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.35892 > ip6-localhost.9000: Flags [.], ack 14, win 342, options [nop,nop,TS val 9399884 ecr 9399884], length 0
01:47:06.393999 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.9000 > ip6-localhost.35892: Flags [F.], seq 14, ack 1, win 342, options [nop,nop,TS val 9399884 ecr 9399884], length 0
01:47:06.394008 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet), ethertype IPv6 (0x86dd), length 86: ip6-localhost.9000 > ip6-localhost.35892: Flags [R.], seq 15, ack 1, win 342, options [nop,nop,TS val 9399884 ecr 9399884], length 0

Looking at the comment in https://github.com/sustrik/libdill/blob/master/tcp.c#L123 and the network captures, I'd guess that the FIN gets indeed send out by hdone(). But since shutdown(3) returns immediately without waiting for handshake completion, the following hclose() terminates the socket instantly (RST flag, second tcpdump), giving the other side no chance to respond.

However, if the handshake is given time to be performed completely (first tcpdump), then no RST is send out.

Using msleep() is of course an ugly hack that will not even work reliably, this SO answer might be worth looking into.

vlm commented 7 years ago

I'd recommend disabling setting LINGER to 0, and create some new function that sets some flag on a socket which would make it send RSTs. Something like a new flag to hdone(int fd, bool success, int64_t deadline); which, if success, does one thing, and does another if failure. Such as setting LINGER to 0. Note that close() will not block if linger is non-zero.

sustrik commented 7 years ago

There are 3 scenarios:

  1. Forceful shutwdown (hclose). Shuts down immediately, no guarantees about sending anything (even RST) to the peer.
  2. Orderly shutdow (tcp_close). Sends FIN to peer, then waits till peer reads all the data and closes the connection.
  3. Two step shutdown (hdone+custom stuff+tcp_close). hdone: Send FIN to peer. custom stuff: Read data from peer. tcp_close: Read remaining data (if any is left unread in previous step) any deallocate the socket.

In short, for orderly shutdown, use tcp_close().

Using hclose() generally means that you don't care about the peer and that you are happy to let it fail or maybe hang forever.

sustrik commented 7 years ago

More info here: https://github.com/sustrik/libdill/blob/master/rfc/bsd-socket-api-revamp.md#protocol-termination

vlm commented 7 years ago

Can I use tcp_close() on btls sockets?

sustrik commented 7 years ago

No, there should be a dedicated btls_close() function to close the socket down cleanly.

vlm commented 7 years ago

This does not make this too much sense. The atachable protocols approach should allow some basic signalling to be applicable, unmodified, to the whole class of protocols (such as messaging, or streaming, or all of them at once). In my app level model I don't want to know what kind of protocols I've applied on top of a file descriptor if I want to close it cleanly.

The default might well be an unclean close (as it seems to be right now).

sustrik commented 7 years ago

The way to close hastily is hclose().

The way to close cleanly is <protocol-name>_close().

The reason there is no unified way to do the latter is that protocol initialization/termination is protocol specifc. For example, udp socket may be opened using udp_open(), tcp socket using tcp_listen/tcp_accept/tcp_connect. On the termination side, some protocols may have additional parameters to the close function (e.g. an error code, message to send to the peer, etc.)

vlm commented 7 years ago

Since there's no btls_close() now, can I use btls_detach? I mean, does this seem technically correct at the moment:

void my_close(int fd, bool tls_used, int64_t deadline) {
    if(tls_used) {
        fd = btls_detach(fd, deadline);
        /* If succeeded, correctly close the underlying protocol */
    }
    /* Lower level protocol is TCP, even if TLS is on top. */
    (void)tcp_close(fd, deadline);
    hclose(fd);
}
vlm commented 7 years ago

In this case, please accept a patch https://github.com/sustrik/dsock/pull/40

sustrik commented 7 years ago

Ack. btls doesn't fit the spec at the moment. We should fix it.