On 6/12/07, Remy Maucherat <[EMAIL PROTECTED]> wrote:
Costin Manolache wrote: > In the sandbox version: > - sleep() and setTimeout(int) -> why not sleep(int millis) ? I think it's > confusing to have both and the interactions between them, in > particular setTimeout is marked optional ? It makes sense to have > setTimeout() as a general timeout. setTimeout() is not optional (the javadoc is out of date, sorry), there was an agreement on that earlier. Timeout sets the connection timeout, which is most likely useful even if there are events. It's quite possible sleep could use a timeout argument (I think calling setTimeout is more or less mandatory when using sleep, and OTOH calling setTimout is not as important otherwise).
Ok - then sleep needs the extra argument, and will mean same as Thread.sleep(), but not-blocking ?
- not sure I understand the use case for isReadable()/isWriteable() - when > will this be called ? My understanding was that when > the connection is readable, a callback will be generated with EventType == > READ. Also it's very confusing to mix the 'blocking' in the > isReadable()/isWriteable() - it would be much cleaner to say that the > connection is always non-blocking, and add a method to switch to blocking > mode ( then use the regular servlet methods maybe ). Using the ComentEvent > for both is IMHO dangerous. Although the read event indicates there's data to read, isReadable indicates if it is possible to continue reading.
My understanding was that the InputStream in request is used for actual reading - and available() could do the same thing. What is the difference then between the 2 ? isWriteable indicates if the last write operation managed to write more
than 0 bytes. If the last write wrote 0, then isWriteable will return false, so the servlet knows it should stop writing on this connection for now (since it cannot accept any output at the moment). Later on, the servlet will receive a write event, and can resume writing.
I'm still a bit confused about this - my understanding was that write event means the previous buffers were written, and you can write more. There are some buffers on the OS side as well as buffers on the connector side. What do you mean by 'managed to write more than 0 bytes' - the write in the OutputStream can go to some of the buffers, or to the client. I assume you don't mean the client ( due to TCP delays ). It is possible to add a blockingMode flag, but I've looked into it a
bit, and found out it is not really needed:
I agree, it is cleaner to not mix them.
- If doing synchronous writes inside some event (either a read or callback event, most likely), then both blocking and non blocking mode make sense. Some servlets may prefer to use blocking mode although it could be holding a thread for a while (for example if the idea is only
maybe a 'waitForEvent()' method to allow a servlet to block if he wants to ? Or is sleep() supposed to do this - I'm not sure from the comments if sleep() will block or just triger an event when the interval expires ?
- will sleep() still allow callbacks ( and if not - what will happen with > them )? What's going to happen with callbacks if callback() is not called ? Yes, sleep is meant mostly to handle the reply-later design easily. The servlet calls sleep(), and is supposed to call callback() (asnychronously, most likely) as a result of something. The connection will then wake and the servlet will get a callback event. If callback is not called, the connection will timeout and the servlet will get its timeout event (according to the connector timeout or the one that has been configured for the connection).
You mean sort of 'notify()' - i.e. someone calls callback() and will trigger the servlet to be executed, interrupting any sleep or wait ?
In general ( both versions): > - it would be great to move it from o.a.catalina to org.apache.comet It's a possibility.
I think more comments and examples ( and maybe better names ) would be great. In any case - the sandbox version seems better as an API, I really don't like the arrays and setConfig in the trunk, and certainly don't like having 2 variants. Costin
