synchronized block question...

G

grz01

Hi,

I'm trying to understand this synchronized-block stuff in java...
I have a Struts Action-class with a piece of code I want to have
synchronized,
so it looks like this:

public class ProcessData extends org.apache.struts.action.Action {

private static Integer semaphore = 0;

public ActionForward execute(ActionMapping mapping, ActionForm
form,
HttpServletRequest request, HttpServletResponse response)
throws Exception {

// ...some code...
synchronized (semaphore)
{
// ...more code...
}
//...more code...

}
}

This works, only one client-call at a time will execute the block, and
any other client-calls will hang waiting until it has completed. So
far so good.

But what puzzles me is that --by experimentation-- if I touch the
semaphore object inside the synchronized-block it seems the lock on
the object gets released.

For example, if I put a stmt:
semaphore++ ;
as the first stmt inside the syncronized block, it appears there is no
locking anymore and 2 different client-calls can now execute the block
simultaneously.

Is this really how it's supposed to work?
Or what am I missing?
 
J

Joshua Cranmer

grz01 said:
But what puzzles me is that --by experimentation-- if I touch the
semaphore object inside the synchronized-block it seems the lock on
the object gets released.

For example, if I put a stmt:
semaphore++ ;
as the first stmt inside the syncronized block, it appears there is no
locking anymore and 2 different client-calls can now execute the block
simultaneously.

Is this really how it's supposed to work?
Or what am I missing?

Let's analyze what the statement is doing in better detail. Since
semaphore is an actual Integer object, we need to box and unbox. So the
statement is the same as this:
semaphore = new Integer(semaphore.intValue() + 1);

Now we can see what's going on. Suppose semaphore begins at 0. When the
first call gets to there, it synchronizes on semaphore, an Integer with
a value of 0. It increments the value, which sets the variable to a new
Integer with a value of 1. The next call then comes in, sees that
nothing is holding a lock on the Integer with a value of 1, and then
executes the code.

In short, your code is locking on two different objects.
 
G

grz01

Let's analyze what the statement is doing in better detail. Since
semaphore is an actual Integer object, we need to box and unbox. So the
statement is the same as this:
semaphore = new Integer(semaphore.intValue() + 1);

Now we can see what's going on. Suppose semaphore begins at 0. When the
first call gets to there, it synchronizes on semaphore, an Integer with
a value of 0. It increments the value, which sets the variable to a new
Integer with a value of 1. The next call then comes in, sees that
nothing is holding a lock on the Integer with a value of 1, and then
executes the code.

In short, your code is locking on two different objects.

Thank you, Joshua,

Hmm... I see... so basically, I cannot do an *assignment* to a
variable that is acting as a lock.
When the second call comes, the variable will then point to a
different object, which then doesnt have a lock on it.

I think its clear. Thanks!
 
J

Joshua Cranmer

Joshua said:
In short, your code is locking on two different objects.

Furthermore, use of primitive wrappers as synchronizing statements is
problematic in that wrappers for some values (like 0) would be cached,
so that these are the same object:

public class Test{
public static void main(String... args) {
Integer a = 0;
Integer b = 0;
System.out.println(a == b);
}
}

(I messed up when I said the boxing conversion was |new Integer(x)|; it
should have been |Integer.valueOf(x)|).
 
G

grz01

Furthermore, use of primitive wrappers as synchronizing statements is
problematic in that wrappers for some values (like 0) would be cached,
so that these are the same object:

public class Test{
   public static void main(String... args) {
     Integer a = 0;
     Integer b = 0;
     System.out.println(a == b);
   }

Ugh... this starts looking kinda ugly, what spurious bugs one may run
into here...

Same thing if I assign an empty string to a variable and use it for
locking, I guess?

So a safer method would be to use a value like: new Date()
for semaphore, that is not likely to get covertly duplicated-by-
caching by the JVM then...?
 
J

Joshua Cranmer

grz01 said:
Ugh... this starts looking kinda ugly, what spurious bugs one may run
into here...

Same thing if I assign an empty string to a variable and use it for
locking, I guess?

So a safer method would be to use a value like: new Date()
for semaphore, that is not likely to get covertly duplicated-by-
caching by the JVM then...?

Anything explicitly constructed via new is safe in that two variables
constructed in this manner are guaranteed to be different in terms of
pointer equality.

You generally can't assume that anything else is safe, especially:
* Autoboxed objects. These are cached for the values [-128, 127] for
bytes, shorts, ints, and longs; booleans cache [true, false], characters
[0, 127], and doubles and floats none.
* String literals. Every literal "A" is the same reference (also the
same as their interned representations).
* Interned strings. These are defined to be the same reference.

These are not safe in the sense mentioned above, but they are useful as
synchronization locks:
* Enum values. An enum contains the only instances.
* Class literals.

If you want a semaphore, you might look at Java's semaphore:
<http://java.sun.com/javase/6/docs/api/java/util/concurrent/Semaphore.html>
 
M

Mark Space

grz01 said:
So a safer method would be to use a value like: new Date()
for semaphore, that is not likely to get covertly duplicated-by-
caching by the JVM then...?

Lew gave you some good ideas.

1. make the object "final". That way Java will give you an error if you
try to replace it.

2. the conventional object to lock on when you just need a random object
to lock is Object.

And Joshua said:

3. yes, definitely look at Semaphore, Lock, and other stuff in
java.util.concurrent.

Lastly, consider locking on some object you already have, rather than
making a special object to lock on. I think synchronizing on the class
object is the same as synchronizing on a static object, and much harder
to mess up. (Joshua did mention class literals too, now that I look.)

public class ProcessData extends org.apache.struts.action.Action {

public ActionForward execute(ActionMapping mapping, ActionForm
form,
HttpServletRequest request, HttpServletResponse response)
throws Exception {

// ...some code...
synchronized ( ProcessData.class )
{
// ...more code...
}
//...more code...

}
}
 
L

Lothar Kimmeringer

grz01 said:
Hmm... I see... so basically, I cannot do an *assignment* to a
variable that is acting as a lock.

You can as you can see above with the result you saw.
If you want to do something like you showed, you need
to implement an own class

public class MySemaphore{
private int count;

public void increaseCount(){
count++;
}
public void decreaseCount(){
count--;
}
public int getCount(){
return count;
}
}

That way you can increase the semaphore-counter but don't exchange
the instance that is used as lock.


Regards, Lothar
--
Lothar Kimmeringer E-Mail: (e-mail address removed)
PGP-encrypted mails preferred (Key-ID: 0x8BC3CD81)

Always remember: The answer is forty-two, there can only be wrong
questions!
 
R

Roland de Ruiter

Joshua said:
You generally can't assume that anything else is safe, especially:
* Autoboxed objects. These are cached for the values [-128, 127] for
bytes, shorts, ints, and longs; booleans cache [true, false],
characters [0, 127], and doubles and floats none.

Other values may be cached; those are the ones guaranteed to be. (Well,
not the longs, but the others are guaranteed. Longs are likely to be
cached, too, though.)

As specified by the JLS:
"If the value p being boxed is true, false, a byte, a char in the range
\u0000 to \u007f, or an int or short number between -128 and 127, then
let r1 and r2 be the results of any two boxing conversions of p. It is
always the case that r1 == r2."

<http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#5.1.7>
 
A

Andreas Leitgeb

Roland de Ruiter said:
As specified by the JLS:
"If the value p being boxed is true, false, a byte, a char in the range
\u0000 to \u007f, or an int or short number between -128 and 127, then
let r1 and r2 be the results of any two boxing conversions of p. It is
always the case that r1 == r2."
<http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#5.1.7>

Thus, sun's implementation of Long$LongCache was optional and not
required by the JLS. Based on previous discussions about object
reuse versus create&throwaway, and also on attempts to cache certain
objects in some program of mine and its measured negative impact on
performance, I'm a bit surprised that they did so without need.

Oh, according to
<http://java.sun.com/javase/6/docs/api/java/lang/Long.html#valueOf(long)>
sun do believe that caching Long instances in an array is likely to
improve performance significantly.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,994
Messages
2,570,223
Members
46,814
Latest member
SpicetreeDigital

Latest Threads

Top