gvanem / Watt-32

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

Check non-blocking connect() status in select_s() #80

Closed jwt27 closed 1 year ago

jwt27 commented 1 year ago

Alternative to #79.

Work in progress - not yet tested!

gvanem commented 1 year ago

I've just restarted the AppVeoyr job on this PR. My bad.

jwt27 commented 1 year ago

OK, this works for me, but I don't quite like the idea of using so_error to keep track of the connection state. SS_ISCONNECTING seems a better choice, but that is being set/cleared in a few other places already. Will see if I can find a solution.

gvanem commented 1 year ago

OK, this works for me,

You mean for the C example you posted for PR-79?

jwt27 commented 1 year ago

OK, this works for me,

You mean for the C example you posted for PR-79?

Yes, and the asio http_client example too.

jwt27 commented 1 year ago

I checked what Linux does. It never sets EINPROGRESS/EALREADY in SO_ERROR (it is always 0 after connect()), so we probably shouldn't do that either. I'm curious to know how *BSD handles it though.

Also I'm wondering about tcp_sock_daemon(). It toggles the connection status flags, but why? Isn't it only for cleanup?

jwt27 commented 1 year ago

Now using SS_ISCONNECTING, I think this is a much better solution.

I also checked how Linux handles send()/recv() after a non-blocking connect. It fails with EAGAIN, and you can repeatedly call it until the connection succeeds. But if you clear the non-blocking flag after connect(), then send()/recv() will wait for the connect to finish. I copied that behavior here.

The last commit isn't 100% related to this issue, but it produces more specific errnos if we get an ICMP reject or the connection is reset. It also prevents SO_ERROR from being ignored.

gvanem commented 1 year ago

. It toggles the connection status flags, but why?

Again, a long time since I wrote that. Can't remember the rationale before running a test-program. Perhaps tests/pull-request-79.c?

gvanem commented 1 year ago

I copied that behavior here.

Good job. I'll merge this soon.

But it would be nice if you/me could test this with some real-life programs. Like Wget, curl or Lynx if you're up for it. I have private Watt-32 makefiles for all these. But for Watt-32 on Windows. I'm in no position to run djgpp on a DOS/Win-XP PC or an emulator. How do you run djgpp programs?

jwt27 commented 1 year ago

. It toggles the connection status flags, but why?

Again, a long time since I wrote that. Can't remember the rationale before running a test-program. Perhaps tests/pull-request-79.c?

I think you meant for that to handle the non-blocking connect. Don't really see what it would do otherwise. The 500ms daemon tick rate seems rather slow for that though, so I moved the logic to _sock_pending_connect(), which also makes sure SO_ERROR is set to something meaningful when SS_ISCONNECTING is cleared.

I copied that behavior here.

Good job. I'll merge this soon.

But it would be nice if you/me could test this with some real-life programs. Like Wget, curl or Lynx if you're up for it. I have private Watt-32 makefiles for all these. But for Watt-32 on Windows. I'm in no position to run djgpp on a DOS/Win-XP PC or an emulator. How do you run djgpp programs?

I cross-compile on Debian or Win7, and then wget the binaries from a DOS machine. If you could send me those makefiles, I'll see if I can get some of those programs running then. Thanks!

jwt27 commented 1 year ago

I got Lynx compiled but I'm not able to run it. Something about ncurses failing to initialize.

Wget (latest version) won't compile, throws a long list of errors.

gvanem commented 1 year ago

I got Lynx compiled but I'm not able to run it. Something about ncurses failing to initialize.

I'm using S-Lang in my home-brewed Makefile.Windows. And Lynx/Watt-32 still works fine AFAICS.

And for curl, I use these Makefiles:

unzip curl-Watt-32.zip
cd lib
make -f Makefile.Windows USE_WATT32=1 
cd ..\src
make -f Makefile.Windows USE_WATT32=1 

But using it; curl.exe -v --wdebug www.watt-32.net, I get the error this PR tries to solve ...

As for Wget, I've added an old version of it here. For Windows, use:

cd bin\Wget.182
nmake -f makefile.vc6 clean all
wget_vc.exe --wdebug -d -r -np www.curl.se

But since OpenSSL is not built-in (it's possible), it's not very useful.

gvanem commented 1 year ago

I wrote> Good job. I'll merge this soon.

Merged and tested with Lynx, Curl and Wget 1.82. Seems to work fine still. libcurl works much better:

curl www.watt-32.net/index.html --remote-name
lynx -dump index.html
...
Watt-32 tcp/ip Homepage

Or from a bin/wget.182/makefile.vc6 build:

wget_vc.exe -d www.watt-32.net
lynx -dump index.html
...
Watt-32 tcp/ip Homepage
gvanem commented 1 year ago

Wget (latest version) won't compile, throws a long list of errors.

Presumably since the real <winsock*.h> messes things up. Using CFLAGS=-I$(WATT_ROOT)/inc/w32-fakes ... and adding -I$(WATT_ROOT)/inc ahead of Gnulib's headers could work.

jwt27 commented 1 year ago

Thanks for merging!

I got Lynx compiled but I'm not able to run it. Something about ncurses failing to initialize.

I'm using S-Lang in my home-brewed Makefile.Windows. And Lynx/Watt-32 still works fine AFAICS.

Just tried S-Lang but the latest version didn't want to compile for djgpp.

And for curl, I use these Makefiles:

What version do these apply to? I did get the latest version to build (just with configure). Seems to work but it's extremely slow (3KB/s).

Wget (latest version) won't compile, throws a long list of errors.

Presumably since the real <winsock*.h> messes things up. Using CFLAGS=-I$(WATT_ROOT)/inc/w32-fakes ... and adding -I$(WATT_ROOT)/inc ahead of Gnulib's headers could work.

Doesn't seem to be Watt-related, it's just not compatible with djgpp ("please port gnulib to your platform" etc). I see there is an msdos/config.h file now but I'm not sure how to use it in a cross compile.