nonblocking mode of sockets is busted in many ways

[bji]

nonblocking Sockets are nearly non-functional. Looking just at the implementation in Socket.cpp:

• socket_send says that it will "Send up to [len] bytes ...", but if it is able to write less than that number of bytes because the write is partial and would block, it doesn't check for EWOULDBLOCK, it just immediately throws a blocking error. Thus it can never actually send less than [len] bytes.
• socket_recv similarly says that it will "Read up to [len]" bytes, but similarly, it does not check for EWOULDBLOCK and throws an error if it can't read exactly [len] bytes.

This means that just about any read or write operation on nonblocking Sockets will always fail.

I haven't looked at the rest of the implementation but it's likely that it suffers from many other issues.

[delahee]

You might want to detail what you expect a bit more because there already is a lot of production code using those :) Le 1 août 2014 20:21, "bji" notifications@github.com a écrit :

nonblocking Sockets are nearly non-functional. Looking just at the implementation in Socket.cpp:

• socket_send says that it will "Send in to [len] bytes ...", but if it is able to write less than that number of bytes because the write is partial and would block, it doesn't check for EWOULDBLOCK

— Reply to this email directly or view it on GitHub https://github.com/HaxeFoundation/hxcpp/issues/92.

[bji]

I suspect that no production code uses nonblocking sockets, because such code clearly cannot work.

[bji]

One thing to note is that I am using the sys.net.Socket.write(content:String) and sys.net.Socket.read() functions. The documentation for these functions is sparse but they do not indicate that they will always fail for nonblocking sockets.

I can see where write() could be construed as possibly throwing a Blocking error, because you can't be guaranteed to be able to write all of the String out before blocking. So perhaps for write() there is just a documentation error where the documentation should indicate that it does not work with nonblocking sockets.

However, I can see no reason that read() needs to fail. It should return whatever partial String is available. It is an implementation error in hxcpp that Socket.read() on nonblocking socket always fails.

[bji]

Also note that even the Output of a nonblocking socket cannot be used properly. Even Output.writeBytes(), which is supposed to write as much as possible, throws a blocking exception for nonblocking sockets if not all can be written. So although Socket.write() can be forgiven for throwing Blocking if not all can be written, Output.writeBytes() cannot.

[delahee]

I read the code https://github.com/HaxeFoundation/hxcpp/blob/6416f8d81920cb10084101475477cda68526a20f/project/libs/std/Socket.cp and, https://github.com/HaxeFoundation/haxe/blob/development/std/cpp/_std/sys/net/Socket.hx and I don't see any filters over the C stdlib that would enforce what you say so the overral behaviour is that of C no more not much less.

Usually socket.wouldBlock pops out when you try to read socket content and there is some tcp buffer overflow of very specific scenario ( http://www.scottklement.com/rpg/socktut/nonblocking.html) and there are very specific ways to control and use them wisely ( select, polling for buffer exhaustion)

And I am fairly sure that this code works in production since openfl use them : https://github.com/openfl/openfl-native/blob/master/flash/net/Socket.hx and we deployed it on a heapload of platforms across thousands (...if not millions of users)...

So maybe a mistake was made on haxe side but maybe not :)

So ahem.. would you please be kidn enough to exhibit an invalid sample to we can examine and if need be unit test these ?

Thanks !

2014-08-01 20:48 GMT+02:00 bji notifications@github.com:

Also note that even the Output of a nonblocking socket cannot be used properly. Even Output.writeBytes(), which is supposed to write as much as possible, throws a blocking exception for nonblocking sockets if not all can be written. So although Socket.write() can be forgiven for throwing Blocking if not all can be written, Output.writeBytes() cannot.

— Reply to this email directly or view it on GitHub https://github.com/HaxeFoundation/hxcpp/issues/92#issuecomment-50920835.

David Elahee

[delahee]

"OpenFL for the C++ target does not use haxe Socket. It provides a URLLoader API that uses its own socket implementation internally."

I meant we use flash.net.socket for direct tora connection on mobile...

"That paragraph doesn't even make any sense"

I was making very generic assertions, waitForRead and btw select are used in production, don't get hot bro ;)

I think we never fell on the case because we always use the fastSend with select so we might have never reached your use case.

Thanks a lot for finding the problem and good continuation !

2014-08-01 21:39 GMT+02:00 bji notifications@github.com:

OpenFL for the C++ target does not use haxe Socket. It provides a URLLoader API that uses its own socket implementation internally.

I don't know what you mean by "filters over the C stdlib". The simple fact is that there is an API that the Socket class is supposed to adhere to, and it's fairly underspecified, but even within the realm of being underspecified, the behavior of the C implementation cannot possibly be correct. How does it make any sense to include a nonblocking mode in the socket API, if that nonblocking mode is guaranteed to fail for all socket reads and writes?

"Usually socket.wouldBlock pops out when you try to read socket content and there is some tcp buffer overflow of very specific scenario ( http://www.scottklement.com/rpg/socktut/nonblocking.html) and there are very specific ways to control and use them wisely ( select, polling for buffer exhaustion)"

That paragraph doesn't even make any sense. "would block" just means that you have tried to read data from a TCP socket that has no data available. That's has nothing to do with buffer overflow on a read. Yes, "would block" on a write means that the socket output buffer is full, but that's only relevant to writes, not reads. "polling" for buffer exhaustion is of course what you are supposed to do, and that's why Haxe Socket provides a select() function. But what is the value of that if you can never know how many bytes you can attempt to write to a Socket before you'll get a blocking error thrown, without any way to know how many bytes actually got written? That's how the C++ implementation is written. It is unusable.

To be frank, although unit tests are certainly needed in this code, they are not needed to demonstrate this bug. The simplest, most cursory examination of the code reveals the flaws. Writing the simplest program using nonblocking Sockets demonstrates the problem. I will provide one shortly, although it will not be a full-on "Unit test".

— Reply to this email directly or view it on GitHub https://github.com/HaxeFoundation/hxcpp/issues/92#issuecomment-50926122.

David Elahee

[bji]

class BadSockets
{
    public static function main()
    {
        var host = new sys.net.Host("localhost");
        var port = 4567;

        var server = new sys.net.Socket();
        var client = new sys.net.Socket();
        var accepted : sys.net.Socket = null;

        server.setBlocking(false);

        server.bind(host, port);
        server.listen(10);

        client.connect(host, port);
        client.setBlocking(false);

        var readSockets = [ server, client ];
        var writeSockets = [ client ];

        var clientMessageNumber = 0;
        var serverMessageNumber = 0;

        while (true) {
            var result =
                sys.net.Socket.select(readSockets, writeSockets, null, null);
            for (w in result.read) {
                if (w == server) {
                    if (accepted == null) {
                        accepted = server.accept();
                        if (accepted == null) {
                            throw "Server accept returned null";
                        }
                        accepted.setBlocking(false);
                        readSockets.push(accepted);
                        writeSockets.push(accepted);
                    }
                    else {
                        throw "Server got another client?";
                    }
                }
                else if (w == client) {
                    trace("client read: [" + client.read() + "]");
                }
                else if (w == accepted) {
                    trace("server read: [" + accepted.read() + "]");
                }
                else {
                    throw "Unknown read socket";
                }
            }
            for (w in result.write) {
                if (w == client) {
                    var msg = "message from client: " + clientMessageNumber++;
                    client.write(msg);
                    trace("client wrote: " + msg);
                }
                else if (w == accepted) {
                    var msg = "message from server: " + serverMessageNumber++;
                    accepted.write(msg);
                    trace("server wrote: " + msg);
                }
                else {
                    throw "Unknown write socket";
                }
            }
        }
    }
}

[bji]

./cpp-BadSockets/BadSockets 
BadSockets.hx:59: client wrote: message from client: 0
Error : Blocking

[ncannasse]

Could you confirm that CPP version matches Neko one here? https://github.com/HaxeFoundation/neko/blob/master/libs/std/socket.c It does indeed throw Blocking if no data could be sent, usually that means that the output buffer is full, but it might also be that you have not finished connecting if you have disabled blocking mode before a connect()

[ncannasse]

Your actual issue is that read() will read the whole data. on a blocking socket that would block until the connection is closed. on a non blocking socket that will always raise Blocking. You instead have to use input.readBytes which will return the number of bytes readed, and then make sure you correctly manage your buffer data.

[ncannasse]

@Simn we might want either fix read() to catch+return on Blocking or document this behavior in the API

[bji]

Someone will have to examine the code in detail to get it all fixed, and I'm not signing up for that at the moment, but here are the basic problems:

In the C++ implementation:

Socket.read() calls:

var bytes:haxe.io.BytesData = socket_read(__s);

socket_read does the following:

    while( true ) {
        POSIX_LABEL(read_again);
        len = recv(sock,buf,256,MSG_NOSIGNAL);
        if( len == SOCKET_ERROR ) {
            HANDLE_EINTR(read_again);
            return block_error();
        }
        if( len == 0 )
            break;
            gc_exit_blocking();
        buffer_append_sub(b,buf,len);
            gc_enter_blocking();
    }

If the socket is nonblocking, this loop will continue to run until it gets an EWOULDBLOCK error, that it then throws as a Blocking error. Therefore, Socket.read() will always throw Blocking on a nonblocking socket.

Socket.write() does this:

    socket_write(__s, haxe.io.Bytes.ofString(content).getData() );

and socket_write looks like this:

    while( datalen > 0 ) {
        POSIX_LABEL(write_again);
        slen = send(sock,cdata,datalen,MSG_NOSIGNAL);
        if( slen == SOCKET_ERROR ) {
            HANDLE_EINTR(write_again);
            return block_error();
        }
        cdata += slen;
        datalen -= slen;
    }

For nonblocking sockets, any one of the send() calls may fail with EWOULDBLOCK, which results in a Blocking throw.

The problem with both of these is that a Blocking exception ends up happening in 'normal' circumstances, and it cannot be recovered from. In the Socket.read case, any bytes read will never be returned, a Blocking exception will always result. In the Socket.write case, some bytes may have successfully been written, but the Blocking exception that occurs does not indicate how many bytes were successfully written so there is no way to continue to later attempt to write the remainder to the socket.

There is a question of how Socket.read and Socket.write should work for nonblocking sockets. Well, I propose:

• Socket.read should read as much as is available and return that amount, which could be 0. It should never throw a Blocking exception.
• Socket.write should loop, writing as much as it can, and when it can't write anymore because of EWOULDBLOCK, enter a select() function to wait on the socket being writable, then continuing. This means that Socket.write() is blocking even on nonblocking sockets, but there is no other way that the API of "writes all of the bytes before returning" can be satisfied. The alternative is to remove the function altogether, or document that it cannot be used when the Socket is in nonblocking mode and immediately throw an exception if used when in nonblocking mode.

[ncannasse]

read() and write() are meant to be quick shortcuts for non blocking sockets. We could change read() to catch Blocking, but for write() we don't want to change it since it would require returning the number of bytes actually written, in which case you should use socket.output.writeBytes which does just that.

[TheSHEEEP]

Is this fixed by now or is this still a problem?

[kiroukou]

It would be great to improve the documentation about this. It's quite hard to get into that API which currently has no documentation or examples, and which does have some shortcuts with specific behaviours... (read/write in non blocking mode).

Thomas

[hughsando]

I've shifted this over to the haxe repo, unless there is something I need to do on the hxcpp side. https://github.com/HaxeFoundation/haxe/issues/5131