You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Adam Fisk <ad...@gmail.com> on 2007/06/22 17:37:20 UTC
Tutorial on ProtocolCodecFilter, state and threads
I've got a quick question for Maarten and anyone else on the snippet on
threads and IoSession attributes from tutorial on ProtocolCodecFilter. The
tutorial reads as follows:
--- start quote --
"We store the state of the decoding process in a session attribute. It would
also be possible to store this state in the Decoder object itself but this
has several disadvantages:
every IoSession would need its own Decoder instance
MINA ensures that there will never be more than one thread simultaneously
executing the decode() function for the same IoSession, but it does not
guarantee that it will always be the same thread. Suppose the first piece of
data is handled by thread-1 who decides it cannot yet decode, when the next
piece of data arrives, it could be handled by another thread. To avoid
visibility problems, you must properly synchronize access to this decoder
state (IoSession attributes are stored in a ConcurrentHashMap, so they are
automatically visible to other threads)."
-- end quote --
If only one thread executes decode() at a time, though, doesn't that mean
the "thread-1" will have already finished its decode() call when more data
arrives and the next thread tries to handle it? If so, I would think any
complicated synchronization issues would not arise because anything that
happens within the decode() method is only happening on one thread at a
time.
I ask because it seems like each IoSession having its own decoder instance
with its own internal state is a pretty good idea!
Am I missing something?
Thanks a lot.
-Adam
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Adam Fisk <a...@lastbamboo.org>.
Anyone else have thoughts on this issue? It still seems like a key part of
understanding MINA, and I do think the Maarten's overall excellent and very
helpful tutorial is misleading as it stands on this point.
-Adam
On 6/23/07, Maarten Bosteels <mb...@gmail.com> wrote:
>
> On 6/23/07, Adam Fisk <a...@lastbamboo.org> wrote:
> >
> > Hi Maarten-
> >
> > Thanks for the link to that discussion thread. It seems like there's
> > quite
> > a bit of confusion on this point, though. The thread really didn't
> reach
> > a
> > common consensus in my reading of it.
> >
> >
> > > Indeed, I want thread-2 to see the changes made by thread-1.
> > > But without synchronization, there is no such guarantee.
> >
> >
> > Right, but the tutorial snippet says "MINA ensures that there will never
> > be
> > more than one thread simultaneously executing the decode() function for
> > the
> > same IoSession". For that to be true, there's gotta be some
> > synchronization
> > somewhere. I decided to dig into the code a little bit for the
> > 1.1.0branch, and indeed ProtocolCodecFilter has the following:
> >
> > try
> > {
> > synchronized( decoderLock )
> > {
> > decoder.decode( session, in, decoderOut );
> > }
> > }
> >
> > With the code above, the changes made by thread-1 on the decoder *are
> > guaranteed to be seen by thread-2*.
>
>
> Indeed.
>
> Now, I realize this is an
> > implementation detail of 1.1.0, but the tutorial description above seems
> > to
> > be relying on that detail.
>
>
> The fact that "no two threads will execute decode() for the same
> IoSession"
> is not an implementation detail, it's a guarantee of the framework.
>
> The use of syncronized(decoderLock) is an implementation detail, but I
> can't
> immediately find
> an alternative implementation that would cause visibility problems in your
> decoder but still
> guarantee that "no two threads will execute decode() for the same
> IoSession"
>
> so maybe that paragraph in the tutorial is not totally accurate.
> Mina experts, what do you think about it ?
>
> Still, I think it's worth reminding people about the danger of the
> visibility problem in general.
>
> So it seems like it should be totally fine to
> > have a ProtocolCodecFactory that returns a new decoder on each
> getDecoder
> > call (effectively one decoder per session). I still don't see the
> > disadvantage of doing this.
>
>
> I guess you're right.
> But the difference between one decoder per session or one decoderState per
> session is small in my eyes.
>
> It certainly seems less complicated that way!
> >
> > Thanks again for your patience Maarten. Seems like a key point we
> should
> > all be clear on, so that's why I'm trying to dig a little deeper here.
>
>
> Right. Let's see what the rest thinks about it.
>
> Maarten
>
> -Adam
> >
>
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Maarten Bosteels <mb...@gmail.com>.
On 6/23/07, Adam Fisk <a...@lastbamboo.org> wrote:
>
> Hi Maarten-
>
> Thanks for the link to that discussion thread. It seems like there's
> quite
> a bit of confusion on this point, though. The thread really didn't reach
> a
> common consensus in my reading of it.
>
>
> > Indeed, I want thread-2 to see the changes made by thread-1.
> > But without synchronization, there is no such guarantee.
>
>
> Right, but the tutorial snippet says "MINA ensures that there will never
> be
> more than one thread simultaneously executing the decode() function for
> the
> same IoSession". For that to be true, there's gotta be some
> synchronization
> somewhere. I decided to dig into the code a little bit for the
> 1.1.0branch, and indeed ProtocolCodecFilter has the following:
>
> try
> {
> synchronized( decoderLock )
> {
> decoder.decode( session, in, decoderOut );
> }
> }
>
> With the code above, the changes made by thread-1 on the decoder *are
> guaranteed to be seen by thread-2*.
Indeed.
Now, I realize this is an
> implementation detail of 1.1.0, but the tutorial description above seems
> to
> be relying on that detail.
The fact that "no two threads will execute decode() for the same IoSession"
is not an implementation detail, it's a guarantee of the framework.
The use of syncronized(decoderLock) is an implementation detail, but I can't
immediately find
an alternative implementation that would cause visibility problems in your
decoder but still
guarantee that "no two threads will execute decode() for the same IoSession"
so maybe that paragraph in the tutorial is not totally accurate.
Mina experts, what do you think about it ?
Still, I think it's worth reminding people about the danger of the
visibility problem in general.
So it seems like it should be totally fine to
> have a ProtocolCodecFactory that returns a new decoder on each getDecoder
> call (effectively one decoder per session). I still don't see the
> disadvantage of doing this.
I guess you're right.
But the difference between one decoder per session or one decoderState per
session is small in my eyes.
It certainly seems less complicated that way!
>
> Thanks again for your patience Maarten. Seems like a key point we should
> all be clear on, so that's why I'm trying to dig a little deeper here.
Right. Let's see what the rest thinks about it.
Maarten
-Adam
>
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Adam Fisk <a...@lastbamboo.org>.
Belated thanks!
-Adam
On 7/16/07, Maarten Bosteels <mb...@gmail.com> wrote:
>
> Adam, Trustin,
>
> I added your comments to the tutorial.
>
> Maarten
>
> On 7/16/07, Trustin Lee <tr...@gmail.com> wrote:
> >
> > Hi Adam,
> >
> > On 6/23/07, Adam Fisk <a...@lastbamboo.org> wrote:
> > > That's also how AsyncWeb is implemented, incidentally, with a separate
> > > encoder and decoder stored as a session attribute for each
> session. The
> > > code for HttpServerCodecFactory does this with the following:
> > >
> > > public ProtocolDecoder getDecoder() throws Exception {
> > > (topLevelState creation omitted)
> > > return new StateMachineProtocolDecoder(topLevelState);
> > > }
> > >
> > > public ProtocolEncoder getEncoder() throws Exception {
> > > return new OneShotHttpResponseEncoder();
> > > }
> > >
> > > This seems preferable to me, honestly, as it's just less complicated.
> >
> > Yes, it is a matter of preference and the characteristic of protocols.
> > I also prefer to create a new decoder per session. However, the
> > decoder of some protocols doesn't need to store any state information,
> > so it might be more suitable to use the same decoder instance for
> > multiple sessions (e.g. UDP).
> >
> > HTH,
> > Trustin
> >
> > > On 6/22/07, Adam Fisk <a...@lastbamboo.org> wrote:
> > > >
> > > > Hi Maarten-
> > > >
> > > > Thanks for the link to that discussion thread. It seems like
> there's
> > > > quite a bit of confusion on this point, though. The thread really
> > didn't
> > > > reach a common consensus in my reading of it.
> > > >
> > > >
> > > > > Indeed, I want thread-2 to see the changes made by thread-1.
> > > > > But without synchronization, there is no such guarantee.
> > > >
> > > >
> > > > Right, but the tutorial snippet says "MINA ensures that there will
> > never
> > > > be more than one thread simultaneously executing the decode()
> function
> > for
> > > > the same IoSession". For that to be true, there's gotta be some
> > > > synchronization somewhere. I decided to dig into the code a little
> > bit for
> > > > the 1.1.0 branch, and indeed ProtocolCodecFilter has the following:
> > > >
> > > > try
> > > > {
> > > > synchronized( decoderLock )
> > > > {
> > > > decoder.decode( session, in, decoderOut );
> > > > }
> > > > }
> > > >
> > > > With the code above, the changes made by thread-1 on the decoder
> *are
> > > > guaranteed to be seen by thread-2*. Now, I realize this is an
> > > > implementation detail of 1.1.0, but the tutorial description above
> > seems
> > > > to be relying on that detail. So it seems like it should be totally
> > fine to
> > > > have a ProtocolCodecFactory that returns a new decoder on each
> > getDecoder
> > > > call (effectively one decoder per session). I still don't see the
> > > > disadvantage of doing this.
> > > >
> > > > It certainly seems less complicated that way!
> > > >
> > > > Thanks again for your patience Maarten. Seems like a key point we
> > should
> > > > all be clear on, so that's why I'm trying to dig a little deeper
> here.
> > > >
> > > > -Adam
> > > >
> > > >
> > >
> >
> >
> > --
> > what we call human nature is actually human habit
> > --
> > http://gleamynode.net/
> > --
> > PGP Key ID: 0x0255ECA6
> >
>
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Trustin Lee <tr...@gmail.com>.
On 7/16/07, Maarten Bosteels <mb...@gmail.com> wrote:
> Adam, Trustin,
>
> I added your comments to the tutorial.
Thanks, Maarten!
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Maarten Bosteels <mb...@gmail.com>.
Adam, Trustin,
I added your comments to the tutorial.
Maarten
On 7/16/07, Trustin Lee <tr...@gmail.com> wrote:
>
> Hi Adam,
>
> On 6/23/07, Adam Fisk <a...@lastbamboo.org> wrote:
> > That's also how AsyncWeb is implemented, incidentally, with a separate
> > encoder and decoder stored as a session attribute for each session. The
> > code for HttpServerCodecFactory does this with the following:
> >
> > public ProtocolDecoder getDecoder() throws Exception {
> > (topLevelState creation omitted)
> > return new StateMachineProtocolDecoder(topLevelState);
> > }
> >
> > public ProtocolEncoder getEncoder() throws Exception {
> > return new OneShotHttpResponseEncoder();
> > }
> >
> > This seems preferable to me, honestly, as it's just less complicated.
>
> Yes, it is a matter of preference and the characteristic of protocols.
> I also prefer to create a new decoder per session. However, the
> decoder of some protocols doesn't need to store any state information,
> so it might be more suitable to use the same decoder instance for
> multiple sessions (e.g. UDP).
>
> HTH,
> Trustin
>
> > On 6/22/07, Adam Fisk <a...@lastbamboo.org> wrote:
> > >
> > > Hi Maarten-
> > >
> > > Thanks for the link to that discussion thread. It seems like there's
> > > quite a bit of confusion on this point, though. The thread really
> didn't
> > > reach a common consensus in my reading of it.
> > >
> > >
> > > > Indeed, I want thread-2 to see the changes made by thread-1.
> > > > But without synchronization, there is no such guarantee.
> > >
> > >
> > > Right, but the tutorial snippet says "MINA ensures that there will
> never
> > > be more than one thread simultaneously executing the decode() function
> for
> > > the same IoSession". For that to be true, there's gotta be some
> > > synchronization somewhere. I decided to dig into the code a little
> bit for
> > > the 1.1.0 branch, and indeed ProtocolCodecFilter has the following:
> > >
> > > try
> > > {
> > > synchronized( decoderLock )
> > > {
> > > decoder.decode( session, in, decoderOut );
> > > }
> > > }
> > >
> > > With the code above, the changes made by thread-1 on the decoder *are
> > > guaranteed to be seen by thread-2*. Now, I realize this is an
> > > implementation detail of 1.1.0, but the tutorial description above
> seems
> > > to be relying on that detail. So it seems like it should be totally
> fine to
> > > have a ProtocolCodecFactory that returns a new decoder on each
> getDecoder
> > > call (effectively one decoder per session). I still don't see the
> > > disadvantage of doing this.
> > >
> > > It certainly seems less complicated that way!
> > >
> > > Thanks again for your patience Maarten. Seems like a key point we
> should
> > > all be clear on, so that's why I'm trying to dig a little deeper here.
> > >
> > > -Adam
> > >
> > >
> >
>
>
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Trustin Lee <tr...@gmail.com>.
Hi Adam,
On 6/23/07, Adam Fisk <a...@lastbamboo.org> wrote:
> That's also how AsyncWeb is implemented, incidentally, with a separate
> encoder and decoder stored as a session attribute for each session. The
> code for HttpServerCodecFactory does this with the following:
>
> public ProtocolDecoder getDecoder() throws Exception {
> (topLevelState creation omitted)
> return new StateMachineProtocolDecoder(topLevelState);
> }
>
> public ProtocolEncoder getEncoder() throws Exception {
> return new OneShotHttpResponseEncoder();
> }
>
> This seems preferable to me, honestly, as it's just less complicated.
Yes, it is a matter of preference and the characteristic of protocols.
I also prefer to create a new decoder per session. However, the
decoder of some protocols doesn't need to store any state information,
so it might be more suitable to use the same decoder instance for
multiple sessions (e.g. UDP).
HTH,
Trustin
> On 6/22/07, Adam Fisk <a...@lastbamboo.org> wrote:
> >
> > Hi Maarten-
> >
> > Thanks for the link to that discussion thread. It seems like there's
> > quite a bit of confusion on this point, though. The thread really didn't
> > reach a common consensus in my reading of it.
> >
> >
> > > Indeed, I want thread-2 to see the changes made by thread-1.
> > > But without synchronization, there is no such guarantee.
> >
> >
> > Right, but the tutorial snippet says "MINA ensures that there will never
> > be more than one thread simultaneously executing the decode() function for
> > the same IoSession". For that to be true, there's gotta be some
> > synchronization somewhere. I decided to dig into the code a little bit for
> > the 1.1.0 branch, and indeed ProtocolCodecFilter has the following:
> >
> > try
> > {
> > synchronized( decoderLock )
> > {
> > decoder.decode( session, in, decoderOut );
> > }
> > }
> >
> > With the code above, the changes made by thread-1 on the decoder *are
> > guaranteed to be seen by thread-2*. Now, I realize this is an
> > implementation detail of 1.1.0, but the tutorial description above seems
> > to be relying on that detail. So it seems like it should be totally fine to
> > have a ProtocolCodecFactory that returns a new decoder on each getDecoder
> > call (effectively one decoder per session). I still don't see the
> > disadvantage of doing this.
> >
> > It certainly seems less complicated that way!
> >
> > Thanks again for your patience Maarten. Seems like a key point we should
> > all be clear on, so that's why I'm trying to dig a little deeper here.
> >
> > -Adam
> >
> >
>
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Adam Fisk <a...@lastbamboo.org>.
That's also how AsyncWeb is implemented, incidentally, with a separate
encoder and decoder stored as a session attribute for each session. The
code for HttpServerCodecFactory does this with the following:
public ProtocolDecoder getDecoder() throws Exception {
(topLevelState creation omitted)
return new StateMachineProtocolDecoder(topLevelState);
}
public ProtocolEncoder getEncoder() throws Exception {
return new OneShotHttpResponseEncoder();
}
This seems preferable to me, honestly, as it's just less complicated.
-Adam
On 6/22/07, Adam Fisk <a...@lastbamboo.org> wrote:
>
> Hi Maarten-
>
> Thanks for the link to that discussion thread. It seems like there's
> quite a bit of confusion on this point, though. The thread really didn't
> reach a common consensus in my reading of it.
>
>
> > Indeed, I want thread-2 to see the changes made by thread-1.
> > But without synchronization, there is no such guarantee.
>
>
> Right, but the tutorial snippet says "MINA ensures that there will never
> be more than one thread simultaneously executing the decode() function for
> the same IoSession". For that to be true, there's gotta be some
> synchronization somewhere. I decided to dig into the code a little bit for
> the 1.1.0 branch, and indeed ProtocolCodecFilter has the following:
>
> try
> {
> synchronized( decoderLock )
> {
> decoder.decode( session, in, decoderOut );
> }
> }
>
> With the code above, the changes made by thread-1 on the decoder *are
> guaranteed to be seen by thread-2*. Now, I realize this is an
> implementation detail of 1.1.0, but the tutorial description above seems
> to be relying on that detail. So it seems like it should be totally fine to
> have a ProtocolCodecFactory that returns a new decoder on each getDecoder
> call (effectively one decoder per session). I still don't see the
> disadvantage of doing this.
>
> It certainly seems less complicated that way!
>
> Thanks again for your patience Maarten. Seems like a key point we should
> all be clear on, so that's why I'm trying to dig a little deeper here.
>
> -Adam
>
>
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Adam Fisk <a...@lastbamboo.org>.
Hi Maarten-
Thanks for the link to that discussion thread. It seems like there's quite
a bit of confusion on this point, though. The thread really didn't reach a
common consensus in my reading of it.
> Indeed, I want thread-2 to see the changes made by thread-1.
> But without synchronization, there is no such guarantee.
Right, but the tutorial snippet says "MINA ensures that there will never be
more than one thread simultaneously executing the decode() function for the
same IoSession". For that to be true, there's gotta be some synchronization
somewhere. I decided to dig into the code a little bit for the
1.1.0branch, and indeed ProtocolCodecFilter has the following:
try
{
synchronized( decoderLock )
{
decoder.decode( session, in, decoderOut );
}
}
With the code above, the changes made by thread-1 on the decoder *are
guaranteed to be seen by thread-2*. Now, I realize this is an
implementation detail of 1.1.0, but the tutorial description above seems to
be relying on that detail. So it seems like it should be totally fine to
have a ProtocolCodecFactory that returns a new decoder on each getDecoder
call (effectively one decoder per session). I still don't see the
disadvantage of doing this.
It certainly seems less complicated that way!
Thanks again for your patience Maarten. Seems like a key point we should
all be clear on, so that's why I'm trying to dig a little deeper here.
-Adam
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Maarten Bosteels <mb...@gmail.com>.
On 6/22/07, Adam Fisk <a...@lastbamboo.org> wrote:
>
> Thanks for getting back to me Maarten. Responses inline.
>
>
> > It's not because thread-1 has finished its decode call before thread-2
> > starts handling more data,
> > that thread-2- will "see" the changes to the decoderState made by
> > thread-1.
> > This is a consequence of The Java Memory Model (do a google search on
> > this,
> > or try http://g.oswego.edu/dl/cpj/jmm.html)
>
>
> Hmnn...I'm still not clear on this. Don't you want thread 2 to see
> changes
> made by thread 1?
Indeed, I want thread-2 to see the changes made by thread-1.
But without synchronization, there is no such guarantee.
The two threads must use some synchronization mechanism, to ensure that they
see each others changes.
It might seem strange at first, but 'the smart guys' who designed tha Java
Memory Model claim there are good reasons for this.
For one thing, it enables the compiler and the JRE to do some optimizations
(statement reordering and such).
Again, I am not an expert on concurrency, but I can highly recommand
http://jcip.net/
If thread 1 successfully sets the state in the decoder
> for an IoSession, and then thread 2 comes along with more data, you want
> thread 2 to use the state thread 1 set, right? If both thread-1 and
> thread
> 2 can be executing decode() simultaneously, it's a totally different story
> (and Doug Lea's concurrency nuances come into play), but your tutorial
> claims this is not the case.
>
>
> > I thought so too, until people on this mailing list told me about the
> > disadvantages of that approach.
>
>
> Interesting. Were those private messages or messages on the list? I'll
> have to do a little digging if they were on here.
see
http://www.nabble.com/ProtocolDecoderAdapter-and-session-state.-tf3413638.html#a9542168
Maarten
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Adam Fisk <a...@lastbamboo.org>.
Thanks for getting back to me Maarten. Responses inline.
> It's not because thread-1 has finished its decode call before thread-2
> starts handling more data,
> that thread-2- will "see" the changes to the decoderState made by
> thread-1.
> This is a consequence of The Java Memory Model (do a google search on
> this,
> or try http://g.oswego.edu/dl/cpj/jmm.html)
Hmnn...I'm still not clear on this. Don't you want thread 2 to see changes
made by thread 1? If thread 1 successfully sets the state in the decoder
for an IoSession, and then thread 2 comes along with more data, you want
thread 2 to use the state thread 1 set, right? If both thread-1 and thread
2 can be executing decode() simultaneously, it's a totally different story
(and Doug Lea's concurrency nuances come into play), but your tutorial
claims this is not the case.
> I thought so too, until people on this mailing list told me about the
> disadvantages of that approach.
Interesting. Were those private messages or messages on the list? I'll
have to do a little digging if they were on here.
Thanks Maarten.
-Adam
Re: Tutorial on ProtocolCodecFilter, state and threads
Posted by Maarten Bosteels <mb...@gmail.com>.
Hello Adam,
On 6/22/07, Adam Fisk <ad...@gmail.com> wrote:
>
> I've got a quick question for Maarten and anyone else on the snippet on
> threads and IoSession attributes from tutorial on
> ProtocolCodecFilter. The
> tutorial reads as follows:
>
> --- start quote --
> "We store the state of the decoding process in a session attribute. It
> would
> also be possible to store this state in the Decoder object itself but this
> has several disadvantages:
>
> every IoSession would need its own Decoder instance
>
> MINA ensures that there will never be more than one thread simultaneously
> executing the decode() function for the same IoSession, but it does not
> guarantee that it will always be the same thread. Suppose the first piece
> of
> data is handled by thread-1 who decides it cannot yet decode, when the
> next
> piece of data arrives, it could be handled by another thread. To avoid
> visibility problems, you must properly synchronize access to this decoder
> state (IoSession attributes are stored in a ConcurrentHashMap, so they are
> automatically visible to other threads)."
> -- end quote --
>
>
> If only one thread executes decode() at a time, though, doesn't that mean
> the "thread-1" will have already finished its decode() call when more data
> arrives and the next thread tries to handle it? If so, I would think any
> complicated synchronization issues would not arise because anything that
> happens within the decode() method is only happening on one thread at a
> time.
It's not because thread-1 has finished its decode call before thread-2
starts handling more data,
that thread-2- will "see" the changes to the decoderState made by thread-1.
This is a consequence of The Java Memory Model (do a google search on this,
or try http://g.oswego.edu/dl/cpj/jmm.html)
I am not very good at explaining it, but fortunately there is an excellent
book on Java Concurrency: see http://jcip.net/
I ask because it seems like each IoSession having its own decoder instance
> with its own internal state is a pretty good idea!
I thought so too, until people on this mailing list told me about the
disadvantages of that approach.
Maarten
Am I missing something?
>
> Thanks a lot.
>
> -Adam
>