Remy Maucherat wrote:
Filip Hanik - Dev Lists wrote:
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.
Yes, this case is the client signaling the end of the connection. In
some other cases, the server can end the connection gracefully,
although with these sort of long running connections, this is almost
never going to happen.
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.
There is still a timeout. I don't see how the DoS risk is any
different from the regular scenario. Maybe it's smaller.
No, sockets in a poller must not be destroyed.
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.
Ok, so the bug is fairly obvious, but I do not see it :) I will review
your code, but can you revert your commit in the meantime ?
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
If the servlet is not reading data in read, there's going to be a
problem anyway. If you
the bug is that Socket.recbb never gets called when its a Comet event.
Well, it's possible, who knows. However, this is a regular read, which
will trickle down to the socket, and cause the usual recvbb call. If
you are right, then there's indeed something obviously something I am
missing.
The input should be exactly the same as before, except in between
reading on the socket, the socket goes in a poller.
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.
Unfortunately, it looks like it:
+ for (int i=0; i<inputBuffer.activeFilters.length; i++) {
+ //this resets the remaining flag and the content
length on the filter
+ //if we don't do this, then
request.getInputStream.read will return 0
+ if (inputBuffer.activeFilters[i]!=null)
inputBuffer.activeFilters[i].setRequest(request);
+ }
This is a big hack to avoid content delimitation.
I see (and saw before) a simple solution to this
1. add to interface InputFilter.append(byte[]|ByteBuffer|ByteChunk)
but I know you would have a field day if I changed an interface, as the
checkin would be much larger and I would for sure get a -1,
at least with this one I had hoped you took a little consideration
befored vetoed.
In terms of reverting my checkin, can we sit on it over the weekend,
since the current code enabled my async client currently, and the old
code is broken.
We're not in a hurry with the Comet stuff are we.
With the proposed feature set, Tomcat would excel beyond the existing
async implementations in like Jetty or Weblogic, yet it would support
the same as they have today.
So we got a late start, that doesn't mean we can't jump ahead of them,
right ;)
Filip
--
Filip Hanik
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
