Remy Maucherat wrote:
Filip
Hanik - Dev Lists wrote:
Gents,
I have played around with the Comet implementation, fixed a few bugs
and got the initial it working, including async data from both client
and server.
I wanted to you get your input on moving forward with the following
features:
1. If CometServlet.read return false, the adapter should call
CometServlet.end, not CometServlet.error (easy)
This is an EOF on a long running input. I consider it an unexpected end
of the communication = an error. This Comet stuff is only useful for
pushing back data.
2. If CometServlet.read throws an error, then
the adapter should call CometServlet.error (easy)
This is done already.
3. Keep-Alive socket support, when
CometServlet.read returns false, don't close the socket, keep alive
should still work and we should still be able to process more HTTP
requests on that connection. So change the status to comet=false, and
process the next request like a standard HTTP request (medium)
Actually, I think the server should be the one closing the connection.
In other cases, since it's a long running request, discarding the
connection is easier. In HTTP land, the server is always the one in
control of keepalive.
That's correct, however, the current implementation closes the
connection after read returns false, so there is no keepalive
implemented.
4. The ability to close the channel from the
server async (medium) - two ways - a) timeout b) call
back from a separate thread
This is too complex/risky: you don't know if the socket is still in the
poller, and destroying it twice or putting back / writing to a
destroyed socket is fatal.
not doing it, means you are setting yourself up for a DOS attack, since
you can run out of connections, all of them being in polling state.
if the socket is in the poller, shouldn't Socket.destroy get it out of
there.
And then the following steps
1. Create a user guide for the CometServlet usage
2. Create an example in servlet-examples
There is one already ("chat"); it's a bad, but it works more or less,
and I used it for testing a bit. I also tested input using a similar
servlet and telnet, and no problem there.
Let me know your thoughts,
IMO, jumping on something like this is not the way to go.
I thought about it a little bit more, and I have to veto your commit:
read will not return 0 (it's the same as it was before: a blocking
read, so it cannot return 0). I don't understand what your intent is
with resetting the remaining bytes numbers, etc. Also, trying to take
care of programming errors in the servlet is pointless: similar errors
could be made just as well with the regular model, entering infinite
loops in a similar way.
think you need to check the code once more, read is not forever
blocking, CometServlet.read->request.getInputStream().read(buf)
doesn't read the socket inputstream, it reads the inputBuffer, and
since the bug in the code never filled the buffer, so yes, the bug is
fairly obvious. even worse, if you override the CometServlet.read
method, cause you want the dialog to continue even though you didn't
receive a packet, then this is what happens
1. The poller knows there is data to be read from the IO socket
2. It wakes up, gets a worker thread, thread invokes the CometServlet,
3. CometServlet.read returns true
4. the worker thread adds the socket back to the poller
5. the poller will immediately wake up, as it has data, the same data
in the buffer from before
the bug is that Socket.recbb never gets called when its a Comet event.
I've spent a good two days getting myself familiar with the code, so
this isn't a quick fix of any sort, and what I did, actually made the
code work, and added in a fully functional client and server push
model. You've done a great initial job, and I would ask you to
reconsider your veto given the fact that I didn't just pull the fix out
of my pants, I worked on it in a very detail oriented manner.
Filip
--
Filip Hanik
|
