The defenition of select is that the bit in the FD-set is set when read() can be called _without_blocking_ according to the manpage.
That should be read as "five usecs later, there's a high probability that it can still be read without blocking".
The easiest way to get into trouble is if you have some other thread or process that reads the data before you get to it. But I wouldn't want to bet that's the only way to get into trouble.
I'm pretty sure it can hapoen with accept() and write(). Not as sure about read(). But I'd prefer a simple rule "don't use callbacks on blocking sockets" to the hairier rule "don't use write or accept callbacks on blocking sockets, but read callbacks are fine, most of the time on most systems".
/ Niels Möller ()
Previous text:
And you would also like to enforce that in pike, even if it breaks quite a few valid uses?
We never did that kind of things before. Actually, we specially added the set_read_callback with friends functions to _allow_ this kind of things.
If we do not allow read-callbacks with blocking sockets, we might as well remove the methods to set them, since they are bound to cause "problems" (like my quite intentional use of blocking sockets with asynchronous notification when data is available, to avoid read threads).
/ Per Hedbor ()
Previous text:
Well, one use is to have an application that reads from stdin without a read-thread. That is not possible now unless you want to block.
Another use is the one I described:
+---------------+ | Main server | +---------------+ | | +----+ +----+ | dl | | ul | +----+ +----+
This code has been very much simplified to make it readable:
In the main server:
class Daemon { Stdio.File fd; mixed do_command( string name, mixed args ) { int len; fd->write( encode_command( name, args ) ); sscanf( fd->read( 4 ), "%4c", len ); return decode_value( fd->read( len ) ); } }
In the daemons:
void got_command( ) { int len; sscanf( fd->read( 4 ), "%4c", len ); mixed res = do_command( decode_command( fd->read( len ) ) ); res = encode_value( res ); fd->write( sprintf("%4c%s", strlen(res), res ) ); }
int main() { fd = Stdio.File( 4, "rw" ); fd->set_read_callback( got_command ); // .. go on to set up other things, like a port to listen to, taskt // to do etc .. return -1; }
/ Per Hedbor ()
Previous text:
I don't have a really strong opinion on that, but I think it makes sense to make it difficult to get into that mode of operation by mistake.
I expect the backend not to block, ever, so it is fair if the backend (or some other part of the callback machinery) complains if I'm using it in such a way that it no longer can guarantee that.
I have no problem if there's a documented way to say "do what I say, I'll take the responsibility of designing my protocols and clients in such a way that they won't deadlock", but it's not something I'll recommend in general, as it puts part of the responsibility for server security (that the server shouldn't hang) onto the clients.
Where possible, the pike interface should hide the difficulties and subtle problems of the operating system services.
/ Niels Möller ()
Previous text:
Here's a read callback that will block the backend for an hour:
void my_read_callback(string data) { sleep(3600); }
Horrible. How should we change the language to ensure that this program is impossible to write? Who cares if we break a lot of legitimate programs, as long as we fix this terrible "bug" in the backend?
/ Marcus Comstedt (ACROSS) (Hail Ilpalazzo!)
Previous text:
The point of the interface the backend/callback stuff is that it shouldn't block on waiting for _i/o_ on one fd while there's i/o possible on other fd:s. The point is to coordinate the i/o blocking for all fd:s. And perhaps signals (although it would be nicer if it didn't have to do that). No more, no less.
When possible, it should be robust to stupidity outside of the process itself (such as stdin being shared with other processes). That you can do arbitrarily stupid things *within* a particular callback function for a particular file is not relevant for the backend machinery.
I think this discussion is getting stupid. Sure, If you feel like implementing a check that emits some kind of warning if sleep is called inside a read callback, go ahead, I'll probably not object, but that is a different issue.
/ Niels Möller ()
Previous text:
The check doesn't even provide the intended protection against races completely, as I noted earlier. I think the discussion has shown that that particular attempt to detect the error wasn't a good one, so I've reverted it.
I'll instead be looking into moving the callback code completely to the Stdio.Fd class, so that the race is avoided better. Nothing except Stdio.File should be depending on the Stdio.Fd interface, hopefully.
/ Martin Stjernholm, Roxen IS
Previous text:
Yes, that's the intention. At least poll/select and the following read should be within the same interpreter lock, but I think it would be more clean if the indirection through the Pike callbacks in Stdio.File is removed completely. But I guess there's some reason for it. Could someone explain what it is?
/ Martin Stjernholm, Roxen IS
Previous text:
If I remember correctly the intention is for it to be a 1-1 mapping between files.Fd objects and fd's. files.Fd_ref exists to make assign(), dup() and similar operations possible (ie several files.Fd_ref objects can refer to the same fd).
/ Henrik Grubbström (Lysator)
Previous text:
Didn't I add a flag to always do "peek" in the read callback?
I needed it in a system that switched between blocking calls to read and non-blocking read callbacks, and it was possible to get a read callback after the socket was emptied with a previous blocking read. I'm sure there's a discussion about the problem in Infokom.
/ Mirar
Previous text:
You did, but it isn't enabled by default, it doesn't provide full protection against a race, and it hurts performance. Do you mean a previous blocking read on some systems makes poll/select return on the fd even when the data has been read out already?
/ Martin Stjernholm, Roxen IS
Previous text:
/.../ or that Pike loops over every socket that made poll exit until it calls poll again (which is a good idea, generally?).
That's probably it, judging from your comment at the start of __stdio_read_callback. I see two ways to solve it without resorting to peeking, which apparently is slow:
1. Read out the data from all the fds before calling any callbacks. 2. Read out the data from one of the fds and do the equivalent of read(0,1) on all the others, then call the callback for the one fd only.
The first solution can have tricky compatibility effects in code that reads on an fd from the read callback of another; it will no longer get any data since it has been read already. Otoh, it seems very precarious to write such code. Do you remember where you came across it?
/ Martin Stjernholm, Roxen IS
Previous text:
It's an IPC protocol stack I wrote that can handle and switch between syncroneous, threaded and asynchroneous.
It seems simle to read the data before calling though, both read and read_callback could use the same buffer and read_callback could be avoided to be called if the buffer is empty?
Ie,
poll(..)
foreach (fd_with_read_set,fd) fd->buffer+=fd->read_some();
foreach (fd_with_read_set,fd) if (sizeof(fd->buffer)) { string buf=fd->empty_buffer(); fd->read_callback(buf); }
...
fd::read(...) { string data=fd->empty_buffer(); ... read() }
Does the opposite problem exist, calling write_callback on a blocking buffer that's been filled by another thread?
/ Mirar
Previous text:
Does the opposite problem exist, calling write_callback on a blocking buffer that's been filled by another thread?
I suppose so, but that can't be helped in all cases. The difference is that the read case will hang the backend before the user callback is even called, which makes the hang "internal". However, the write case should have a similar flag as you've suggested for read, so that Pike at least can guarantee that there's write buffer available at the moment when the write callback is called.
/ Martin Stjernholm, Roxen IS
Previous text:
Then the socket has to be set blocking after select() has finished. If that happens due to a race then the check isn't effective since the race can just as well occur after it and still cause a later read to block. I'd still like to have some sort of safeguard against blocking operations from the backend.
/ Martin Stjernholm, Roxen IS
Previous text:
We've seen it block on the line
string s=::read(8192,1);
in __stdio_read_callback.
So you mean the only situation when that can happen is if some other thread has managed to make a read before it? Then the real problem is probably that the read doesn't take place on the C level while the interpreter lock is held. What about moving all the stuff done in the pike level callbacks into Stdio.Fd, so that we can do away with them completely?
/ Martin Stjernholm, Roxen IS
Previous text:
Yes, it is not supposed to be possible for read() to block when select (or poll) has reaturned that data is available.
I can only see two situations that can cause that:
1> The socket is available in two different processes, and the other process gets there first.
2> We actually call read twice from this process, either by using threads, or because the FD is present twice in the fd-set (or list of pollfds), or perhaps OOB-data is read separately, causing this problem?
/ Per Hedbor ()
Previous text:
It's an http connection in Roxen, so OOB isn't involved.
The first situation shouldn't be possible in a normal Pike process that spawns processes through the Process.* functions, should it?
Is anything using Stdio.Fd directly? I think the only sane thing is to extend it so that it has the data available before entering any pike code at all.
/ Martin Stjernholm, Roxen IS
Previous text:
pike-devel@lists.lysator.liu.se