RE: The synchronized keyword is used too early in the processActionCreate() method of the RequestProcessor class

[Donnie Hale <do...@haleonline.net>]

See below.

>
> You mentioned that the three steps are atomic. To me, atomic means that
> they all succeed or the operation set fails. I don't think this
> is the case
> here. If the lookup finds an action, the other operations are not
> executed.
> To me, the atomic part is, if the action is not found in the map:
> 1)       Start Atomic operation.
>     2) Create an instance of the Action.
>     3) Store the instance in the Map.
>     4) Stop atomic operation.
>
> I believe this is the set of operations that should be atomic. By
> ensuring
> that these operations succeed without interruption, is the main goal.

First, I mean atomic here in the sense that they all have to occur within a
sychronized block, not in the database sense of atomicity, succeed-fail etc.

Second, I'll try to more carefully explain the race condition that can occur
if the synchronization doesn't begin before checking the map. I'll use your
scenario, which in pseudocode is:

1) Action actn = map.get(actionType);
2) if (actn == null) {
3)     synchronized {
4)         Action actn = new actionType;
5)         map.put(actionType, actn);
6)     }
   }
   return actn;

We have two threads, "a" and "b" arriving at this code sequence, racing to
get through; and both threads are interested in the same action type.

Thread "a" executes 1) and 2); then a context switch occurs. At that point,
there is still not an Action of the appropriate type in the map.

So thread "b" executes 1), 2), and 3); it's got the lock now.

Thread "a" wakes up and tries to do 3), but can't so it blocks.

Thread "b" wakes up again. It executes 4), 5), and 6); now there's an Action
of the appropriate type in the map, and thread "b" goes merrily on its way.

Thread "a" wakes back up and can now obtain the lock. It executes 3), 4),
5), and 6). So a second Action of the same action type has been put int the
map; since HashMap only allows one for a given key, it overwrites the first
Action that thread "b" had put in there.

Again, in this narrow scenario, it may be acceptable to live with that since
getting a second one that's functionally equivalent to the first, and
letting the first get gc'd, isn't a big deal. Another option would be to
make the map functionally read-only by creating all the Actions at
initialization time rather than "just-in-time". Not sure of the implications
of that idea, though.

This precise scenario is one reason why the whole concept of "thread-safe"
containers like the original HashTable are a bad idea in my opinion. Sure,
it may make access to the internal data structures safe for the duration of
any single method call on the object; but it doesn't help any with the
logical scenarios via which a container is accessed.

>
> As far as double-check pattern goes, I don't believe moving synchronized
> down just to around the action create changes anything with respect to
> double-check. As far as I understand it, the double-check problem
> has to do
> with certain optimizing compilers having the freedom to rearrange the
> creation during a constructor call, and the actual setting of the field
> reference of the new object that was just created in the constructor. If
> one thread calls the constructor and an object is created, another thread
> may check the field reference and not see it because the compiler has
> rearranged the execution.

I only mentioned the double-check pattern because someone might say that
that would be a valid optimization, and I wanted to preemptively respond to
that. :)

Hope that all helps,

Donnie


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

RE: The synchronized keyword is used too early in the processActionCreate() method of the RequestProcessor class

[Chuck Cavaness <ch...@attbi.com>]

Donnie,

   Very good explantation. I get it now. Thanks for taking the time.

Chuck

At 10:07 PM 4/15/2002 -0400, you wrote:
>See below.
>
> >
> > You mentioned that the three steps are atomic. To me, atomic means that
> > they all succeed or the operation set fails. I don't think this
> > is the case
> > here. If the lookup finds an action, the other operations are not
> > executed.
> > To me, the atomic part is, if the action is not found in the map:
> > 1)       Start Atomic operation.
> >     2) Create an instance of the Action.
> >     3) Store the instance in the Map.
> >     4) Stop atomic operation.
> >
> > I believe this is the set of operations that should be atomic. By
> > ensuring
> > that these operations succeed without interruption, is the main goal.
>
>First, I mean atomic here in the sense that they all have to occur within a
>sychronized block, not in the database sense of atomicity, succeed-fail etc.
>
>Second, I'll try to more carefully explain the race condition that can occur
>if the synchronization doesn't begin before checking the map. I'll use your
>scenario, which in pseudocode is:
>
>1) Action actn = map.get(actionType);
>2) if (actn == null) {
>3)     synchronized {
>4)         Action actn = new actionType;
>5)         map.put(actionType, actn);
>6)     }
>    }
>    return actn;
>
>We have two threads, "a" and "b" arriving at this code sequence, racing to
>get through; and both threads are interested in the same action type.
>
>Thread "a" executes 1) and 2); then a context switch occurs. At that point,
>there is still not an Action of the appropriate type in the map.
>
>So thread "b" executes 1), 2), and 3); it's got the lock now.
>
>Thread "a" wakes up and tries to do 3), but can't so it blocks.
>
>Thread "b" wakes up again. It executes 4), 5), and 6); now there's an Action
>of the appropriate type in the map, and thread "b" goes merrily on its way.
>
>Thread "a" wakes back up and can now obtain the lock. It executes 3), 4),
>5), and 6). So a second Action of the same action type has been put int the
>map; since HashMap only allows one for a given key, it overwrites the first
>Action that thread "b" had put in there.
>
>Again, in this narrow scenario, it may be acceptable to live with that since
>getting a second one that's functionally equivalent to the first, and
>letting the first get gc'd, isn't a big deal. Another option would be to
>make the map functionally read-only by creating all the Actions at
>initialization time rather than "just-in-time". Not sure of the implications
>of that idea, though.
>
>This precise scenario is one reason why the whole concept of "thread-safe"
>containers like the original HashTable are a bad idea in my opinion. Sure,
>it may make access to the internal data structures safe for the duration of
>any single method call on the object; but it doesn't help any with the
>logical scenarios via which a container is accessed.
>
> >
> > As far as double-check pattern goes, I don't believe moving synchronized
> > down just to around the action create changes anything with respect to
> > double-check. As far as I understand it, the double-check problem
> > has to do
> > with certain optimizing compilers having the freedom to rearrange the
> > creation during a constructor call, and the actual setting of the field
> > reference of the new object that was just created in the constructor. If
> > one thread calls the constructor and an object is created, another thread
> > may check the field reference and not see it because the compiler has
> > rearranged the execution.
>
>I only mentioned the double-check pattern because someone might say that
>that would be a valid optimization, and I wanted to preemptively respond to
>that. :)
>
>Hope that all helps,
>
>Donnie
>
>
>--
>To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
>For additional commands, e-mail: <ma...@jakarta.apache.org>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>