From: Daniel Pitts on 25 Mar 2010 12:50 On 3/24/2010 5:42 PM, Roedy Green wrote: > On Wed, 24 Mar 2010 15:55:30 -0700, markspace<nospam(a)nowhere.com> > wrote, quoted or indirectly quoted someone who said : > >> If you later export that local reference to another thread or object, it >> should be fine. It's valid to synchronize on a object that some other >> part of the system will see later. > > I have a JTable. I get a row put it in a local variable and > synchronise on that. Does that not lock anyone else getting that > row, even if they have nothing to do with my local variable? > > AppToWatch row; > synchronized ( ALL_ROWS ) > { > row = ALL_ROWS.get( rowIndex ); > state = row.getState(); > } > .... > synchronized ( row ) > { > url = row.getVersionURL(); > marker = row.getMarker(); > } I can't say for sure without seeing more of the code, but this looks like a road to deadlock. -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
From: Daniel Pitts on 25 Mar 2010 15:23 On 3/25/2010 11:35 AM, Roedy Green wrote: > On Thu, 25 Mar 2010 11:01:30 -0700 (PDT), Lew<lew(a)lewscanon.com> > wrote, quoted or indirectly quoted someone who said : > >>> it is a bit fat for a SSCCE. >> >> Well, the point of an SSCCE is that you trim down the "fat" code to a >> minimal case that illustrates the problem. Part of the value in doing >> that is that you often find the problem in the course of doing the >> trimming. > > The problem is any piece of code with a JTable in it and TableModel is > still going to be obese. Do you believe your problem is because of those classes? If not, you can write an SSCCE that doesn't use them. -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
From: Daniel Pitts on 25 Mar 2010 15:28 On 3/25/2010 11:06 AM, Eric Sosman wrote: > On 3/25/2010 1:29 PM, Roedy Green wrote: >> On Wed, 24 Mar 2010 16:21:29 -0700, Patricia Shanahan<pats(a)acm.org> >> wrote, quoted or indirectly quoted someone who said : >> >>> It's difficult to know if the message was valid or not without an SSCCE. >> >> you can see the complete code at >> https://wush.net/websvn/mindprod/listing.php?repname=mindprod&path=/com/mindprod/vercheck/ >> >> >> it is a bit fat for a SSCCE. > > Would have been nice of you to point out which of the twelve > source files elicited the complaint ... For others who may want > a look, VerCheck.java seems to be the culprit. > > Roedy, I still don't know what's going on, but there's at least > one synchronization problem apparent in the code: > > for ( int rowIndex = 0; rowIndex < ALL_ROWS.size(); rowIndex++ ) > ... > synchronized ( ALL_ROWS ) > { > row = ALL_ROWS.get( rowIndex ); > > As the comment just before this loop says, rows might be deleted > from ALL_ROWS while this is running, meaning that rowIndex might > be out of range by the time you call get(). > > This doesn't appear closely related to a complaint about > synchronizing on row -- but I've often found, when confronted > by a mysterious problem, that it pays to clean up *all* the > other issues, even those that are "obviously unrelated." > > By the way, if ALL_ROWS can be perturbed while the loop is > in progress, the iteration may miss rows (visit row 4, other > thread deletes row 2 and slides everything down one slot, visit > row 5 which used to be row 6, never visit the old row 5), or may > visit a row more than once (visit row 4, other thread inserts a > row after row 2 and slides everything up, visit row 5 which is > the same row just visited as 4). The comment indicates you don't > want to lock ALL_ROWS for the entire loop, but to keep the > iteration self-consistent you might want to do something like > lock ALL_ROWS, grab a snapshot with toArray(), unlock, and run > the iteration on the (private, stable) snapshot. I agree except, don't use toArray, use new ArrayList<Row>(); Or, instead of all the sync, check, blah-de-blah, use the standard EDT mechanisms for modifying/accessing this table. That way, you're actions are all serial, and you don't have to worry so much. Threading isn't that hard to get right, but it isn't that hard to get wrong either. The only truly difficult part of writing MT code is that people aren't taught how to look at it and determine whether it is right or not. -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
From: Roedy Green on 25 Mar 2010 17:27 On Thu, 25 Mar 2010 11:01:30 -0700 (PDT), Lew <lew(a)lewscanon.com> wrote, quoted or indirectly quoted someone who said : >It's very difficult to reason about locks and synchronization when >they depend on changeable references, They don't. The two variables are final in the real code. I "simplified". My bad. I think the problem is just that in general local variables can be subject to casual modification, perhaps during a later patch, which would break the lock. -- Roedy Green Canadian Mind Products http://mindprod.com If you tell a computer the same fact in more than one place, unless you have an automated mechanism to ensure they stay in sync, the versions of the fact will eventually get out of sync.
From: Roedy Green on 25 Mar 2010 17:27
On Wed, 24 Mar 2010 15:11:15 -0700, Roedy Green <see_website(a)mindprod.com.invalid> wrote, quoted or indirectly quoted someone who said : >The IntelliJ code Inspector (lint) slapped my wrist for synchronising >on a local variable. What's the problem with that? The sync is on >the object, not the reference, right? I got this response from an IntelliJ employee (probably a Russian with ESL). The point was synchronization only makes sense if two different threads syncing on the same instance and question arises how safe was an exchange, that two threads have same reference on their stacks. Most obviously "clean" subject to synchronize on is a final field. Synchronizing on a local reference might well be not a problem but generally not that easy to verify for correctness and easy thing to broke later. -- Roedy Green Canadian Mind Products http://mindprod.com If you tell a computer the same fact in more than one place, unless you have an automated mechanism to ensure they stay in sync, the versions of the fact will eventually get out of sync. |