gvanem / Watt-32

Watt-32 TCP/IP library and samples.
https://www.watt-32.net/
18 stars 8 forks source link

Don't set EISCONN at first connect #79

Closed jwt27 closed 1 year ago

jwt27 commented 1 year ago

Linux manpage for connect() states that SO_ERROR will be 0 when a connection is established on a non-blocking socket. See description below EINPROGRESS:

https://man7.org/linux/man-pages/man2/connect.2.html

After select(2) indicates writability, use getsockopt(2) to read the SO_ERROR option at level SOL_SOCKET to determine whether connect() completed successfully (SO_ERROR is zero)

I think Watt32 should do the same. It makes sense: establishing a connection is not an error.

Also, I think a small optimization can be made here. EALREADY can only be set on a non-blocking socket, so testing for SS_NBIO is not necessary. (please double-check if that is correct, otherwise drop the second commit)

jwt27 commented 1 year ago

I don't actually see where ETIMEDOUT / ECONNREFUSED is set for non-blocking sockets. Seems to be only done in connect()? Shouldn't select() handle this?

gvanem commented 1 year ago

I don't actually see where ETIMEDOUT / ECONNREFUSED is set for non-blocking sockets.

I don't remember. I wrote that many years ago. But is there a problem with an Asio test-program you want to solve with these patches? If so, could I have the .cpp-file + Makefile for that?

jwt27 commented 1 year ago

I first noticed the problem when I tried this example:

https://github.com/jwt27/asio/blob/djgpp/asio/src/examples/cpp14/iostreams/http_client.cpp

"http" in s.connect(...) should be changed to "80" (I guess we don't have /etc/services). There is a configure script, but you can also compile it via:

i386-pc-msdosdjgpp-g++ -I/path/to/watt/inc -I/path/to/asio/include -DWATT32_NO_OLDIES -DASIO_DISABLE_THREADS http_client.cpp -o http -lwatt

BUT: I just realized, you will need a patched libc, since asio uses ioctl() which is (still) broken.


So maybe it's easier to demonstrate with a C example:

#include <sys/socket.h>
#include <netinet/in.h>
#include <sys/ioctl.h>
#include <poll.h>
#include <stdio.h>
#include <unistd.h>

#ifndef __DJGPP__
# define ioctlsocket ioctl
#endif

static uint32_t ip(char a, char b, char c, char d)
{
  return (d << 24) | (c << 16) | (b << 8) | (a);
}

int main()
{
  int fd = socket(PF_INET, SOCK_STREAM, 0);
  if (fd < 0)
  {
    perror("socket()");
    return 1;
  }

  int opt = 1;
  if (ioctlsocket(fd, FIONBIO, (char*)&opt) < 0)
  {
    perror("ioctlsocket()");
    goto fail;
  }

  struct sockaddr_in addr;
  addr.sin_family = AF_INET;
  addr.sin_addr.s_addr = ip(/* enter some ip here */);
  addr.sin_port = htons(/* enter some port here */);

  if (connect(fd, (struct sockaddr*)&addr, sizeof(struct sockaddr_in)))
  {
    if (errno != EINPROGRESS)
    {
      perror("connect()");
      goto fail;
    }
  }

  struct pollfd pfd;
  pfd.fd = fd;
  pfd.events = POLLOUT;
  pfd.revents = 0;
  if (poll(&pfd, 1, -1) < 0)
  {
    perror("poll()");
    goto fail;
  }

  int so_error;
  socklen_t len = sizeof(int);
  if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &so_error, &len))
  {
    perror("getsockopt()");
    goto fail;
  }

  if (so_error != 0)
  {
    printf("CONNECT FAILED: %d\n", so_error);
    goto fail;
  }

  printf("connected!\n", so_error);
  close(fd);
  return 0;

fail:
  close(fd);
  return 1;
}

Result of SO_ERROR should be 0 if connect succeeded, or otherwise EHOSTUNREACH/ETIMEDOUT/ECONNREFUSED.

Instead it returns EISCONN (error) on success, and if it fails then poll() will block forever. Only EHOSTUNREACH is correctly detected. edit: seems it does time out eventually, but the error is EALREADY.

jwt27 commented 1 year ago

Closing in favour of #80.

gvanem commented 1 year ago

So maybe it's easier to demonstrate with a C example:

Is the above test program still specific to the issue and valid for PR #80 ?

BTW, this program builds only with djgpp. I've added this modified test for it. Running it as pull-request-79.exe -d, I get CONNECT FAILED: 113/already connected. And the wattcp.dbg shows that the TCP handshake is completed. But select_s() immediately drops the connection. From wattcp.sk:

00:00:00.008: socket: fam:AF_INET type:STREAM, proto IPPROTO_TCP, 3
00:00:00.008: ioctlsocket:3, file cmd: FIONBIO 1
00:00:00.008: connect:3, auto-binding, src/dest ports: 1066/80, EINPROGRESS (-1)
00:00:00.008: poll: num 1, to -1
00:00:00.009:   select_s: n=0-3, rwx, timeout undef, cnt=1 (0r/1w/0x)
00:00:00.045:   rc 1
00:00:00.045: getsockopt:3, SO_ERROR, val 113=errno 113
00:00:00.046: close_s:3