Socket connect() timeout facility needed

[star297]

Description of defect

No control of LWIP connect() timeout.

TCPSocket needs a timeout.

@kjbracey-arm

Re issue #13056

In folder: connectivity\lwipstack\source\LWIPStack.cpp

nsapi_error_t LWIP::socket_connect(nsapi_socket_t handle, const SocketAddress &address)
{
    struct mbed_lwip_socket *s = (struct mbed_lwip_socket *)handle;
    ip_addr_t ip_addr;

    nsapi_addr_t addr = address.get_addr();
    if (!convert_mbed_addr_to_lwip(&ip_addr, &addr)) {
        return NSAPI_ERROR_PARAMETER;
    }

    //+++++++++++++++!! this is the problem !!+++++++++++++
    netconn_set_nonblocking(s->conn, false);
    //++++++++++++++++
    err_t err = netconn_connect(s->conn, &ip_addr, address.get_port());
    netconn_set_nonblocking(s->conn, true);

    return err_remap(err);
}

Could you guys remove the 'netconn_set_nonblocking(s->conn, false);' please?
To allow Socket.set_timeout(); to work.
Or something to that effect.
I believe the default is 30 seconds, I need and use timeouts in the order of 10-200ms.

I have to change this on every OS update and can't use Mbed-Online.

Target(s) affected by this defect ?

All targets that would want to use LWIP

Toolchain(s) (name and version) displaying this defect ?

Latest Mbed Studio and On-Line toolchains.

What version of Mbed-os are you using (tag or sha) ?

mbed-os-5.15.0
mbed-os-6.6.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Mbed Studio
Mbed Studio-Online
Mbed OnLine

How is this defect reproduced ?

Calling a connect() timeout of 100ms

my_socket.set_timeout(100);

has no effect on the default connect() timeout that appears to be around 30 seconds.

[ciarmcom]

@star297 thank you for raising this issue.Please take a look at the following comments:

How can we reproduce your issue?

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'. This indicates to us that at least all the fields have been considered.
Please update the issue header with the missing information, the issue will not be mirrored to our internal defect tracking system or investigated until this has been fully resolved.

[star297]

Run this and try to connect to a Null or dead IP address:

    Timer t1;
    TCPSocket remote_device;
    SocketAddress remote_addr("some IP",some Port);
    remote_device.open(net);        

    remote_device.set_timeout(10);  // set 10 mS timeout
    
    t1.start();  
    remote_device.connect(remote_addr); 
    t1.stop();
    printf ("connect time: %f\n", t1.read());

The connect time will always be 30 seconds no matter what timeout is set

[ciarmcom]

@star297 it has been 5 days since the last reminder. Could you please update the issue header as previously requested?

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-3149

[0xc0170]

cc @ARMmbed/mbed-os-connectivity

[kjbracey]

Background to this is in #13056. From discussion there, just deleting the "nonblocking false" line wasn't sufficient - the "nonblocking true" had to be pulled up into its place. Has that analysis changed? Oh, I think it has because of #13205. So after that, both "nonblocking" calls in connect can be removed, right?

This would make the Mbed OS Socket timeout+blocking APIs work on the connect call with lwIP, as they already do for Nanostack and maybe other stacks. This is a bug fix, but there is likely to be application breakage. Apps that have only been run with lwIP may be setting a socket to non-blocking (for reads and writes) between the open and connect calls, but expect the initial connect itself to be blocking.

That's why the LWIPStack code is as it currently is - we saw apps that did that and thus worked on lwIP but failed on Nanostack. Our own apps have been updated, so should still work on lwIP after this change, but third party lwIP-only apps could still fail, so we shied away from revising LWIP::socket_connect.

If you want to avoid potential app breakage you may want to gate the change on a config flag. Maybe have a lwip.connect-always-blocks flag that defaults to true but can be turned off for the enhanced behaviour. (Or reverse that to lwip.connect-obeys-timeouts?) Can't decide on best polarity there - are you activating a "legacy behaviour" or a "feature"?

[star297]

Depreciate the original functionality to allow the timeout would be the normal Mbed way :)

If you do not call the set_timeout(); the default still remains blocking for around 30 seconds.

I don't think you will be able to produce a depreciated message during compile will be the issue though.
So this change must be reflected in the the OS6 Docs.