I'm using synchronized blocks. Is it valid?

[Wilder C Rodrigues]

Hi,
In my lock/unlock methods I used synchronized blocks. Is it valid?
I'll send my assignment friday and I'm so nervous.
Thx,
Wilder

[Jim Yingst]

Sure. There's not much difference between using synchronized blocks and synchronized methods - you can make a valid solution either way. Blocks allow more explicit control, which can be useful in some cases. I wouldn't worry about it. Just make sure that the locking logic works, and is as simple to understand as you can make it.

[Philippe Maquet]

Hi Wilder,
I agree with Jim, just adding that not only it's valid, but it's probably the best solution.
Indeed, synchronizing the whole lock/unlock methods wouldn't make much sense :

Or you have one Data instance per client and your synchronized keyword in lock/unlock methods signatures will have no effect (synchronizing an instance method == synchronized(this), and this cannot be a good candidate to be a monitor in that context)

Or you have only one Data instance shared by all clients and using Data.this as a lock monitor will uselessly slow down any other Data method you'd need to synchronize.

Synchronizing on your locks container is definitely better IMO.
Best,
Phil.

[Mark Spritzler]

Yes, synchronized blocks are fine, actually my lock method used synchronized blocks rather than making the method synchronized.
I am only posting this to re-inforce what those guys have already answered for you.
Mark

[Jim Yingst]

I'm disagreeing a bit with Philippe just for old times' sake...
Or you have only one Data instance shared by all clients and using Data.this as a lock monitor will uselessly slow down any other Data method you'd need to synchronize.
Unless you're using record-level locking instead. The synchronized methods don't necessarily have to be methods of the Data class, and they don't have to be all methods of the Data class. For many solutions with sync blocks, there's a way you could refactor it so that the sync block is actually a separate sync method (either of Data or of some other class). It's hard to say in general whether such refactoring is a good idea or not - if you've got a design that is clear to you now; don't mess with it. But if you've got a design which is convoluted, there's often a way to refactor it so it's clearer.
Another point is that if you use full caching, it's quite possible that the server performance is limited by network performance, serializing and desirializing with RMI, more than any sync effects. In this case, using a little extra "wasteful" sync may not have any noticable effect.
One problem with sync blocks is that they may give you too much power. If you sync on a monitor other than "this", I think you have a greater chance of creating a deadlock if you're not careful. On the other hand, using sync block at least makes you be more explicit about what instance is being used as the monitor. I think many newbies may not realize they need to pay attention to this sort of thing, e.g. to understand the distinction between a static sync block and a nonstatic sync block. Using sync blocks is a bit uglier, and can blow up in your face if you're not careful - but the additional level of explicitness may be a good thing.
So, I'm not really sure I'd recommend one over another in general. Both techniques can work well, or can lead to trouble if not used well.

[Philippe Maquet]

Hi Jim,

I'm disagreeing a bit with Philippe just for old times' sake...

So little.

Using sync blocks is a bit uglier, and can blow up in your face if you're not careful - but the additional level of explicitness may be a good thing.

Agreed at 100%. I read a few posts here where people synchronized Data methods while they had one instance of Data per client. Useless and misleading (except maybe from the grader's perspective).
I synchronize methods too, but when it's clear in the context that "this" makes sense as a monitor.

One problem with sync blocks is that they may give you too much power. If you sync on a monitor other than "this", I think you have a greater chance of creating a deadlock if you're not careful.

Agreed at 99% You're right if you don't apply the solution of using multiple classes you mentioned above : "For many solutions with sync blocks, there's a way you could refactor it so that the sync block is actually a separate sync method (either of Data or of some other class)". With such multiple implicit synchs the risk of deadlock is even higher, because of the implicit way of doing.

So, I'm not really sure I'd recommend one over another in general. Both techniques can work well, or can lead to trouble if not used well.

Yes, definitely.
Best,
Phil.

[Bill Robertson]

Philippe,
you said

I read a few posts here where people synchronized Data methods while they had one instance of Data per client. Useless and misleading (except maybe from the grader's perspective).

Why is this useless and misleading? Or do you mean a new instance of Data
for each client is useless and misleading? (rather than having one instance of data for every client?)

[Philippe Maquet]

Hi Bill,

Why is this useless and misleading? Or do you mean a new instance of Data
for each client is useless and misleading? (rather than having one instance of data for every client?)

By synchronizing Data methods, you implicitly synchronize on Data.this. So if you have one Data instance per client, you're implicitly synchronizing on different monitors which is ... "useless and misleading". I mean there that your method-level synchronized keywords have no effect (none) hence they are useless, and that since they are useless, they are - by definition - misleading : the guy (junior or senior) who reads your sources will think that you are synchronizing some access while in fact you don't.
Best,
Phil.

[Dave Knipp]

Phil has a good point.
You should only be synchronizing on the 'static' member that is consistent across the server and therefore is accesible by all clients. By 'static' i mean the class/container that has your pertinent data in it. ie. if you have a hashmap of lockedrecords. This 'static' member should be what every client references and thus requires synchronization, not Data itself. IMO you would only synchronize Data's methods if you made it a singleton and therefore only had one instance of it per server. What does everyone else think?
Dave

[Bill Robertson]

this is exactly how I have it

[Bill Robertson]

I forgot to mention, I have a sychronized method calling another
synchronized method, this scares me becuase of deadlock issues.
But I don't know a way around it.

[Philippe Maquet]

Hi Bill,
If those synchronized methods belong to the same class, don't care about crossed calls : java the synchronize mechanism is reentrant.
Best,
Phil.

[Bill Robertson]

Phil,

If those synchronized methods belong to the same class, don't care about crossed calls : java the synchronize mechanism is reentrant.

Could you elaborate a little more on this. Especially what you mean about
crossed calls being reentrant. And yes, they all belong to my Data class which is a singleton.
(my weakest subject is locking).
bill

[Jim Yingst]

Alternately, if the sync methods are in different classes, then you can still be safe if you establish a hierarchy of the classes. If classes A and B have sync methods, then you might allow A to call methods of B, but don't let B call methods of A. (The safest way is to make sure B doesn't even have a reference to an A, or to anything else which might have a reference to A.) This will make you safe from deadock. The only problem is, it's not always easy to see what's gong on - if some junior programmer puts a reference to A into class B, then you may get deadlock. So try to document how things work clearly, to minimize this risk.
[ October 23, 2003: Message edited by: Jim Yingst ]

[Philippe Maquet]

Hi Bill,
I'd better have written "don't care about multiple chained synchronized calls".
synchronized methodA() may safely call synchronized methodB() which in turn may safely call synchronized methodC().
When the caller thread calls methodA() it acquires a lock on this. When methodB() and indirectly methodC() are then called from within methodA(), the caller thread simply keeps the lock on this it has already.
Notice also that if you call a non synchronized method from within a synchronized one (even indirectly), the caller thread still keeps its lock. It means that even in the case where all your entry-point methods in a given class (let's say the public ones) would need to be synchronized, other methods which are only called from the latter ones (let's say the private methods) wouldn't need to be synchronized. By synchronizing them uselessly, you just give the JVM a bit more work.
Best,
Phil.
PS What Jim wrote about the multiple classes context is of course right. He completes what I wrote above.
[ October 23, 2003: Message edited by: Philippe Maquet ]

[Bill Robertson]

I get it, thanks a ton.