A correct way of dealing with reads appearing after close

[Alexei Dets]

Hi!

I've noticed that it's a quite common situation when protocol chain is
invoked after connection is already closed or while connection is closing
in parallel to protocol chain processing (SelectionKeyHandler.cancel is
called in another thread, i.e. from
AsyncWriteCallbackHandler.onWriteCompleted).

This usually results in various exceptions (depending how far close process
advanced) and unnecessary creation of a new worker thread's byte buffer,
population of a worker thread attachment attributes, creation of a new
ProtocolParser (if ParserProtocolFilter is involved) that will later be
attached to an already cancelled selection key, sometimes even in
successfull parsing of the protocol message that will be just discarded
because there is no any way to know where it should be delivered (because
BaseSelectionKeyHandler.doAfterKeyCancel removed selection key attachment
and cleared the ThreadAttachment attributes effectively destroying all
connection context information).

So, the question is - is the described behavior normal and what are
the "best practices" to deal with this (or avoid this problem)?

        Alexei


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Oleksiy Stashok]

Hi Alexei,

> I've noticed that it's a quite common situation when protocol chain is
> invoked after connection is already closed or while connection is  
> closing
> in parallel to protocol chain processing (SelectionKeyHandler.cancel  
> is
> called in another thread, i.e. from
> AsyncWriteCallbackHandler.onWriteCompleted).
>
> This usually results in various exceptions (depending how far close  
> process
> advanced) and unnecessary creation of a new worker thread's byte  
> buffer,
> population of a worker thread attachment attributes, creation of a new
> ProtocolParser (if ParserProtocolFilter is involved) that will later  
> be
> attached to an already cancelled selection key, sometimes even in
> successfull parsing of the protocol message that will be just  
> discarded
> because there is no any way to know where it should be delivered  
> (because
> BaseSelectionKeyHandler.doAfterKeyCancel removed selection key  
> attachment
> and cleared the ThreadAttachment attributes effectively destroying all
> connection context information).
>
> So, the question is - is the described behavior normal and what are
> the "best practices" to deal with this (or avoid this problem)?

The described behavior is not so normal, but as you mentioned it's  
common :)
Most of the time the reason, why a connection gets closed on separate  
thread is:
1) Connection was recognized as broken (probably client closed a  
connection)
2) Some timeout exceeded

The best practice here. IMO, is stop processing the broken connection  
and release associated resources ASAP.

WBR,
Alexey.

>
>        Alexei
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@...
> For additional commands, e-mail: users-help@...
>


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Alexei Dets]

Hi!
Oleksiy Stashok wrote:
> The best practice here. IMO, is stop processing the broken connection
> and release associated resources ASAP.

I agree :-)

I guess the question is how to _reliably_ find out that this is a broken
connection? I wasn't able to find any examples of such checks in Grizzly
sources (of course I didn't check every file...). On the contrary I see
that usually framework doesn't do any checks at all - i.e. on dead
connections ReadFilter allocates a new buffer, ParserProtocolFilter creates
a new parser instance, SelectionKeyHandler.cancel closes already closed
connections (and repeats close notifications!). I guess there is a good
reason for this behavior without any checks and synchronizations (i.e. they
can hurt performance a lot) but it is not clear what should be done with
this _in_practice_ on the application level :-(

For example, application can try to introduce some synchronization (though
it can be very difficult as whole Grizzly API is based on selection keys
instead of channels but after introduction of LinuxSpinningWorkaround
selection keys are not persistent anymore) but this will probably affect
performance (and much more compared to possible synchronization inside the
framework). Or it can just ignore all exceptions appearing inside Grizzly -
but is it wise? What is advisable, what is a big "NO-NO"? ;-) It'll be
interesting, and I guess not only for me, to get the
opinion/recommendations of the framework authors and "success stories" of
the framework users.

BTW, "release associated resources ASAP" is another interesting topic - who
will care about this after SelectionKeyHandler.cancel already cleared
ThreadAttachment and SelectionKey but ProtocolChain was called after this
and so its filters re-populated ThreadAttachment & attached it to
SelectionKey? ;-)

        Alexei



---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Oleksiy Stashok]

>> The best practice here. IMO, is stop processing the broken connection
>> and release associated resources ASAP.
>
> I agree :-)
>
> I guess the question is how to _reliably_ find out that this is a  
> broken
> connection?
Here we're limited by JDK. Talking about TCP channels, the most  
reliable way, IMO, is to get -1 or IOException, when trying to read  
data. It's exactly what ReadFilter is doing. If ReadFilter found, that  
connection is closed - it will stop processing and close the connection.

> I wasn't able to find any examples of such checks in Grizzly
> sources (of course I didn't check every file...). On the contrary I  
> see
> that usually framework doesn't do any checks at all - i.e. on dead
> connections ReadFilter allocates a new buffer, ParserProtocolFilter  
> creates
> a new parser instance, SelectionKeyHandler.cancel closes already  
> closed
> connections (and repeats close notifications!).
That's strange. If ReadFilter is able to read data - it should mean  
connection is alive.
As for memory allocation, ReadFilter tries to use already allocated  
buffer, if there is no buffer associated with the current thread - it  
really allocates new one. If error occurs during reading bytes from a  
channel, the buffer will not be lost, it will be reused during the  
next request processing.

> I guess there is a good
> reason for this behavior without any checks and synchronizations  
> (i.e. they
> can hurt performance a lot) but it is not clear what should be done  
> with
> this _in_practice_ on the application level :-(
As I mentioned ReadFilter is the most reliable "check".

> For example, application can try to introduce some synchronization  
> (though
> it can be very difficult as whole Grizzly API is based on selection  
> keys
> instead of channels but after introduction of LinuxSpinningWorkaround
> selection keys are not persistent anymore) but this will probably  
> affect
> performance (and much more compared to possible synchronization  
> inside the
> framework).

Not sure we need synchronization mechanism here. What we will  
synchronize?


> Or it can just ignore all exceptions appearing inside Grizzly -
> but is it wise?
Ignoring all exceptions is not wise, but if it's exception related to  
IOException, what you can do about it? The connection is not operable  
anymore - you can just release associated resources, right?

> What is advisable, what is a big "NO-NO"? ;-) It'll be
> interesting, and I guess not only for me, to get the
> opinion/recommendations of the framework authors and "success  
> stories" of
> the framework users.
>
> BTW, "release associated resources ASAP" is another interesting  
> topic - who
> will care about this after SelectionKeyHandler.cancel already cleared
> ThreadAttachment and SelectionKey but ProtocolChain was called after  
> this
> and so its filters re-populated ThreadAttachment & attached it to
> SelectionKey? ;-)

Not sure this is possible scenario. If connection has been closed, a  
protocol chain will end up on ReadFilter. If you see different  
behavior - it's bug, which should be fixed.
To intercept connection closing - you need to implement  
ConnectionCloseHandler and register it on SelectionKeyHandler.

Hope this will help.

WBR,
Alexey.

>
>        Alexei
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@...
> For additional commands, e-mail: users-help@...
>


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Alexei Dets]

Hi!
Oleksiy Stashok wrote:
> That's strange. If ReadFilter is able to read data - it should mean
> connection is alive.

However, this works only if ReadFilter executes always. It the parsing is
done using ParserProtocolFilter + ProtocolParser then this assumption is
wrong - if large enough (for some logical protocol messages) block of data
was received by ReadFilter then ProtocolChain will be re-invoked and
ParserProtocolFilter will not call ReadFilter.execute. This can be
desirable - if it was remote end that send some data and closed the
connection - we need to read and parse everything, or undesirable - if it
was the application itself that closed the connection - and so it is not
interested anymore in anything that could have left.

> As for memory allocation, ReadFilter tries to use already allocated
> buffer, if there is no buffer associated with the current thread - it
> really allocates new one. If error occurs during reading bytes from a
> channel, the buffer will not be lost, it will be reused during the
> next request processing.

I think that if you'll get a WorkerThread that is associated with a
ThreadAttachment that was reset (in SelectionKeyHandler.cancel) then it'll
allocate a new buffer. And after protocol chain execution completes and
ParserProtocolFilter attaches ThreadAttachment to SelectionKey in
postExecute then ThreadAttachment, SelectionKey, buffer & ProtocolParser
will be pending garbage collection. ProtocolParser will be created because
ThreadAttachment & SelectionKey were cleared in SelectionKeyHandler.cancel
and so ParserProtocolFilter will think that this is a new connection
without a protocol parser. BTW, ProtocolParser will also think that this is
a new connection and will parse accordingly.

> Not sure we need synchronization mechanism here. What we will
> synchronize?

Unfortunately, I have no answer for this one. It just feels wrong then
ProtocolChain execution continues even after connection was explicitly
closed by the app itself. I think there should be some mechanism in place
that interrupts protocol chain execution and data parsing after application
decided that it's done with this connection.

>> BTW, "release associated resources ASAP" is another interesting
>> topic - who
>> will care about this after SelectionKeyHandler.cancel already cleared
>> ThreadAttachment and SelectionKey but ProtocolChain was called after
>> this
>> and so its filters re-populated ThreadAttachment & attached it to
>> SelectionKey? ;-)
> Not sure this is possible scenario.

Definitely it is possible and it happens, see my description of events
above. It is not necessary that they will be exactly the same as
ProtocolChain execution can take place during different stages of
connection closing/clearing code. It's possible that processing will end
with some exception, i.e. ConcurrentModificationException if some filter
will try to manipulate attributes while they are cleared in
SelectionKeyHandler.cancel (actually this was the exception that grabbed my
attention to this behavior).

> If connection has been closed, a
> protocol chain will end up on ReadFilter.

Only if it is executed in the protocol chain...

        Alexei


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...

[Oleksiy Stashok]

Hi Alexei,

That's strange. If ReadFilter is able to read data - it should mean

connection is alive.

However, this works only if ReadFilter executes always. It the parsing is
done using ParserProtocolFilter + ProtocolParser then this assumption is
wrong - if large enough (for some logical protocol messages) block of data
was received by ReadFilter then ProtocolChain will be re-invoked and
ParserProtocolFilter will not call ReadFilter.execute. This can be
desirable - if it was remote end that send some data and closed the
connection - we need to read and parse everything,

Agree

or undesirable - if it
was the application itself that closed the connection - and so it is not
interested anymore in anything that could have left.

Right. This is not implemented.

To achieve this behavior - you'll probably need to extend ParserProtocolFilter and add that logic.

Anyway it's good point, we'd probably need to think about some general solution for this. Will appreciate any help here!

As for memory allocation, ReadFilter tries to use already allocated

buffer, if there is no buffer associated with the current thread - it

really allocates new one. If error occurs during reading bytes from a

channel, the buffer will not be lost, it will be reused during the

next request processing.

I think that if you'll get a WorkerThread that is associated with a
ThreadAttachment that was reset (in SelectionKeyHandler.cancel) then it'll
allocate a new buffer.

And after protocol chain execution completes and
ParserProtocolFilter attaches ThreadAttachment to SelectionKey in
postExecute then ThreadAttachment, SelectionKey, buffer & ProtocolParser
will be pending garbage collection. ProtocolParser will be created because
ThreadAttachment & SelectionKey were cleared in SelectionKeyHandler.cancel
and so ParserProtocolFilter will think that this is a new connection
without a protocol parser. BTW, ProtocolParser will also think that this is
a new connection and will parse accordingly.

From you words it definitely looks like a bug!

Can you pls. just elaborate... As I understand here we're talking about situation, when one thread processes parsing, another one is doing cancel? Can you pls. provide the steps (one-by-one), when the situation above will occur?

Not sure we need synchronization mechanism here. What we will

synchronize?

Unfortunately, I have no answer for this one. It just feels wrong then
ProtocolChain execution continues even after connection was explicitly
closed by the app itself. I think there should be some mechanism in place
that interrupts protocol chain execution and data parsing after application
decided that it's done with this connection.

+1000000 :)

If something needs/could to be fixed on Grizzly side - we should do that.

BTW, "release associated resources ASAP" is another interesting

topic - who

will care about this after SelectionKeyHandler.cancel already cleared

ThreadAttachment and SelectionKey but ProtocolChain was called after

this

and so its filters re-populated ThreadAttachment & attached it to

SelectionKey? ;-)

Not sure this is possible scenario.

Definitely it is possible and it happens, see my description of events
above. It is not necessary that they will be exactly the same as
ProtocolChain execution can take place during different stages of
connection closing/clearing code. It's possible that processing will end
with some exception, i.e. ConcurrentModificationException if some filter
will try to manipulate attributes while they are cleared in
SelectionKeyHandler.cancel (actually this was the exception that grabbed my
attention to this behavior).

Agree, this could happen if you explicitly close a connection on server side. This situation, IMO, is not very normal, but possible.

Though, not sure synchronization (waiting until protocol chain on different thread will finish its work) is good solution here. From other side, protocol chain should not do what it does according to your previous observation.

Hope with your help and details I asked above - we will be able to workaround connection close the best way we can.

If connection has been closed, a

protocol chain will end up on ReadFilter.

Only if it is executed in the protocol chain...

Right.

Thanks.

WBR,

Alexey.

       Alexei


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@...
For additional commands, e-mail: users-help@...