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? -- Roedy Green Canadian Mind Products http://mindprod.com Don�t worry about people stealing an idea; if it�s original, you�ll have to shove it down their throats. ~ Howard Aiken (born: 1900-03-08 died: 1973-03-14 at age: 73)
![]() |
0 |
![]() |
Roedy Green wrote: > 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? Need SSCCE. http://sscce.org/
![]() |
0 |
![]() |
Roedy Green wrote: > 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? Each thread gets its own local variable. Defeats the purpose of synchronizing on one. AHS
![]() |
0 |
![]() |
Arved Sandstrom wrote: > Roedy Green wrote: >> 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? > > Each thread gets its own local variable. Defeats the purpose of > synchronizing on one. 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. If you just synchronized on a local variable and then let it go out of scope, yeah, that's a pretty serious "D'oh!" moment.
![]() |
0 |
![]() |
On 24-03-2010 18:11, Roedy Green wrote: > 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? Local variables are thread specific. Passing on a local variable to another thread via some global variable/singleton/cache would be a pretty bad design. I can understand the complaint. Arne PS: yes - it is the object not the ref.
![]() |
0 |
![]() |
Arne Vajh�j wrote: > On 24-03-2010 18:11, Roedy Green wrote: >> 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? > > Local variables are thread specific. > > Passing on a local variable to another thread via some > global variable/singleton/cache would be a pretty bad design. > > I can understand the complaint. > > Arne > > PS: yes - it is the object not the ref. It may be a case of import, rather than export. Suppose the method for obtaining a reference to some object is fairly expensive, and synchronized. Each thread could have obtained a reference to the same object and cached it in a local variable. It's difficult to know if the message was valid or not without an SSCCE. Patricia
![]() |
0 |
![]() |
On 24-03-2010 19:21, Patricia Shanahan wrote: > Arne Vajh�j wrote: >> On 24-03-2010 18:11, Roedy Green wrote: >>> 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? >> >> Local variables are thread specific. >> >> Passing on a local variable to another thread via some >> global variable/singleton/cache would be a pretty bad design. >> >> I can understand the complaint. > > It may be a case of import, rather than export. Suppose the method for > obtaining a reference to some object is fairly expensive, and > synchronized. Each thread could have obtained a reference to the same > object and cached it in a local variable. It could be. But I would tend to believe that would be messy as well. To be able to easily verify that threads are indeed synchronizing on the same object, then I think it is best if is relative simple to see what is being synchronized on. > It's difficult to know if the message was valid or not without an SSCCE. I don't think the tool does super sophisticated code analysis. Most likely it just have some simple rules and flag things that looks weird. If they are good enough, then the developer can ignore the warning. And a good tool would have the option to store the ignore decision. Arne
![]() |
0 |
![]() |
Roedy Green wrote: > 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? Right, it's not illegal and may be correct. But it may be incorrect as well, and probably is will be unless you can guarantee that the local will point to the same instance in all threads. IntelliJ thinks enough people make that mistake that it's worth warning about. It's similar to while (methodCall()); i++; That's legal and might be exactly what you meant to do, but the first semicolon is likely enough to be spurious that a lint-like tool should say something.
![]() |
0 |
![]() |
markspace wrote: > Arved Sandstrom wrote: >> Roedy Green wrote: >>> 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? >> >> Each thread gets its own local variable. Defeats the purpose of >> synchronizing on one. > > > 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. > > If you just synchronized on a local variable and then let it go out of > scope, yeah, that's a pretty serious "D'oh!" moment. > I think all the responses have pretty much narrowed in on the same things: the sync is indeed on the object, which means that however you obtain/store a reference to that object that everything is using as a lock, it's certainly legal to sync on an object referred to locally. But since it's something that is somewhat unusual, and could (probably) be a mistake, I imagine IntelliJ is flagging it along the lines of what Mike suggested. AHS
![]() |
0 |
![]() |
On Wed, 24 Mar 2010 15:55:30 -0700, markspace <nospam@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(); } -- Roedy Green Canadian Mind Products http://mindprod.com Don�t worry about people stealing an idea; if it�s original, you�ll have to shove it down their throats. ~ Howard Aiken (born: 1900-03-08 died: 1973-03-14 at age: 73)
![]() |
0 |
![]() |
On 24-03-2010 20:42, Roedy Green wrote: > On Wed, 24 Mar 2010 15:55:30 -0700, markspace<nospam@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? It does. But the code would probably be difficult to read and this is why a code style checker flag it. Arne
![]() |
0 |
![]() |
On 3/24/2010 8:42 PM, Roedy Green wrote: > On Wed, 24 Mar 2010 15:55:30 -0700, markspace<nospam@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(); > } Are you sure you didn't misread IntelliJ's complaint? The thing that strikes me as odd about this code is that there are three method calls made on row's object, only two of which are synchronized on that object. It's conceivable that this is all right, but it sure looks strange to me -- and perhaps IntelliJ thinks it peculiar, too. -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
On 3/24/2010 5:42 PM, Roedy Green wrote: > On Wed, 24 Mar 2010 15:55:30 -0700, markspace<nospam@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/>
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 00:06:56 -0400, Eric Sosman <esosman@ieee-dot-org.invalid> wrote, quoted or indirectly quoted someone who said : > Are you sure you didn't misread IntelliJ's complaint? all it says is "synchronization on local variable row". -- 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.
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 00:06:56 -0400, Eric Sosman <esosman@ieee-dot-org.invalid> wrote, quoted or indirectly quoted someone who said : >> AppToWatch row; >> synchronized ( ALL_ROWS ) >> { >> row = ALL_ROWS.get( rowIndex ); >> state = row.getState(); >> } >> ... >> synchronized ( row ) >> { >> url = row.getVersionURL(); >> marker = row.getMarker(); >> } > > Are you sure you didn't misread IntelliJ's complaint? The >thing that strikes me as odd about this code is that there are >three method calls made on row's object, only two of which are >synchronized on that object. It's conceivable that this is all >right, but it sure looks strange to me -- and perhaps IntelliJ >thinks it peculiar, too. I corrected the code to read: AppToWatch row; synchronized ( ALL_ROWS ) { row = ALL_ROWS.get( rowIndex ); synchronized ( row ) { state = row.getState(); } } ... synchronized ( row ) { url = row.getVersionURL(); marker = row.getMarker(); } Now I get TWO message about synchorizing on row. -- 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.
![]() |
0 |
![]() |
On Wed, 24 Mar 2010 16:21:29 -0700, Patricia Shanahan <pats@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. -- 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.
![]() |
0 |
![]() |
On 3/25/2010 1:27 PM, Roedy Green wrote: > [...] > Now I get TWO message about synchorizing on row. A politician would call this "Progress." ;-) Sorry; I'm out of ideas about what IntelliJ dislikes. Let us know if (no, let's be optimistic, "when") you find out; it'll probably be interesting and/or instructive. -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
Roedy Green wrote: > 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. Eric Sosman >> =A0 =A0 Are you sure you didn't misread IntelliJ's complaint? =A0The >> thing that strikes me as odd about this code is that there are >> three method calls made on row's object, only two of which are >> synchronized on that object. =A0It's conceivable that this is all >> right, but it sure looks strange to me -- and perhaps IntelliJ >> thinks it peculiar, too. > Roedy Green wrote: > I corrected the code to read: > > =A0 =A0AppToWatch row; > =A0 =A0synchronized ( ALL_ROWS ) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 row =3D ALL_ROWS.get( rowIndex ); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 synchronized ( row ) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 state =3D row.getState(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0... It's not clear to me how this next section gets its 'row' reference. Presumably it has to synchronize on 'ALL_ROWS' via the above fragment. Regardless, any time you have inconsistent locking protocols, e.g., using two locks in one place and only one in another, you risk having strange problems, so it makes sense that IntelliJ warns you about it. As with "unchecked" warnings, that doesn't mean that you're wrong, necessarily, only that you're in dangerous territory. > =A0 =A0 synchronized ( row ) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 url =3D row.getVersionURL(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 marker =3D row.getMarker(); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > > Now I get TWO message about synchorizing on row. > It's very difficult to reason about locks and synchronization when they depend on changeable references, and involve different locking sequences. I swan the warning is appropriate and you should either live with it or come up with a locking protocol that's easier to understand. -- Lew
![]() |
0 |
![]() |
On 3/25/2010 1:29 PM, Roedy Green wrote: > On Wed, 24 Mar 2010 16:21:29 -0700, Patricia Shanahan<pats@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. -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 11:01:30 -0700 (PDT), Lew <lew@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. -- 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.
![]() |
0 |
![]() |
Roedy Green wrote : > On Thu, 25 Mar 2010 00:06:56 -0400, Eric Sosman > <esosman@ieee-dot-org.invalid> wrote, quoted or indirectly quoted > someone who said : > > I corrected the code to read: Have you tried to explicitly initialise row? AppToWatch row = null; > synchronized ( ALL_ROWS ) > { > row = ALL_ROWS.get( rowIndex ); > synchronized ( row ) > { > state = row.getState(); > } > } > ... > synchronized ( row ) > { > url = row.getVersionURL(); > marker = row.getMarker(); > } > > Now I get TWO message about synchorizing on row. -- Wojtek :-)
![]() |
0 |
![]() |
On 3/25/2010 11:35 AM, Roedy Green wrote: > On Thu, 25 Mar 2010 11:01:30 -0700 (PDT), Lew<lew@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/>
![]() |
0 |
![]() |
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@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/>
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 11:01:30 -0700 (PDT), Lew <lew@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.
![]() |
0 |
![]() |
On Wed, 24 Mar 2010 15:11:15 -0700, Roedy Green <see_website@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.
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 14:06:53 -0400, Eric Sosman <esosman@ieee-dot-org.invalid> wrote, quoted or indirectly quoted someone who said : > 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. Sorry I thought I had pointed you to Vercheck.java. It has since occurred to me I could try concocting a much simpler piece of code without any JTable that still triggered the lint. -- 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.
![]() |
0 |
![]() |
On Thu, 25 Mar 2010 12:28:45 -0700, Daniel Pitts <newsgroup.spamfilter@virtualinfinity.net> wrote, quoted or indirectly quoted someone who said : >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. The compiler rarely complains and the program nearly always run one thread at a time. You can't easily simulate pathological cases the way you can with single thread. Perhaps there is some drug that induces paranoia that could be used once you have your first cut. -- 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.
![]() |
0 |
![]() |
Roedy Green wrote: > I got this response from an IntelliJ employee (probably a Russian with > ESL). Hmm, well that's one strike against IntelliJ then, if they don't hire tech support that can communicate properly.
![]() |
0 |
![]() |
On 25-03-2010 17:27, Roedy Green wrote: > On Wed, 24 Mar 2010 15:11:15 -0700, Roedy Green > <see_website@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. JetBrains is a czech company, so czech not russian sounds more likely. But the text looks fine to me. It is more or less what we have been telling you for two days. Arne
![]() |
0 |
![]() |
On 25-03-2010 15:28, Daniel Pitts wrote: > 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. That applies to other things than MT! :-) Arne
![]() |
0 |
![]() |
On Wed, 24 Mar 2010, Roedy Green wrote: > 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? It's like the warning on the lack of a serialVersionUID in a Serializable class. It isn't necessarily wrong, but there is a popular class of errors that involve doing this. tom -- life finds a way
![]() |
0 |
![]() |
On 3/25/2010 3:28 PM, Daniel Pitts wrote: > On 3/25/2010 11:06 AM, Eric Sosman wrote: >> [... about iterating over a changing List ...] >> 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>(); Not confrontational, just curious: Why prefer a new ArrayList to an array? To me, it appears that an ArrayList is just an array wrapped up in extra machinery, and I can't see that the machinery adds any value for this usage. So, why pay the extra freight? What am I missing? -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
On 3/26/2010 7:22 AM, Eric Sosman wrote: > On 3/25/2010 3:28 PM, Daniel Pitts wrote: >> On 3/25/2010 11:06 AM, Eric Sosman wrote: > >> [... about iterating over a changing List ...] >>> 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>(); > > Not confrontational, just curious: Why prefer a new ArrayList > to an array? To me, it appears that an ArrayList is just an array > wrapped up in extra machinery, and I can't see that the machinery > adds any value for this usage. So, why pay the extra freight? > What am I missing? Array is a relatively primitive concept. You're question is akin to asking why use a "Date object" instead of an "int representing the milliseconds since Jan 1, 1970". Either one "works". Yes, the non-primitive has more "machinery", but that isn't automatically a bad thing. The primitive looses semantic meaning outside of its context, where a properly designed abstraction maintains its semantics regardless of context. Lists are also easier to work with, and work in more places. -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
![]() |
0 |
![]() |
On 3/25/2010 2:27 PM, Roedy Green wrote: > On Wed, 24 Mar 2010 15:11:15 -0700, Roedy Green > <see_website@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. One thing you can do is more the synchronization into the "Row" class, and turn your operations into atomic operations, rather than have external clients need to concern themselves with synchronization. That way, the Row class can always sync on the same object (probably "this", but not necessarily), and the client needn't care about it. -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
![]() |
0 |
![]() |
Daniel Pitts wrote: > On 3/26/2010 7:22 AM, Eric Sosman wrote: >> On 3/25/2010 3:28 PM, Daniel Pitts wrote: >>> On 3/25/2010 11:06 AM, Eric Sosman wrote: >> >> [... about iterating over a changing List ...] >>>> 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>(); >> >> Not confrontational, just curious: Why prefer a new ArrayList >> to an array? To me, it appears that an ArrayList is just an array >> wrapped up in extra machinery, and I can't see that the machinery >> adds any value for this usage. So, why pay the extra freight? >> What am I missing? > Array is a relatively primitive concept. You're question is akin to > asking why use a "Date object" instead of an "int representing the > milliseconds since Jan 1, 1970". > > Either one "works". Yes, the non-primitive has more "machinery", but > that isn't automatically a bad thing. The primitive looses semantic > meaning outside of its context, where a properly designed abstraction > maintains its semantics regardless of context. > > Lists are also easier to work with, and work in more places. > I tend to use an array where the underlying structure I am representing really is an array, and a collection where the underlying structure really is a list or set or whatever. Specifically I am not going to use an ArrayList, for example, if the thing being modelled is not resizable. Why would I want to write code to ensure that the ArrayList will stay at a fixed size when an aray will already take care of that for me? AHS
![]() |
0 |
![]() |
On 3/26/2010 4:15 PM, Daniel Pitts wrote: > On 3/26/2010 7:22 AM, Eric Sosman wrote: >> On 3/25/2010 3:28 PM, Daniel Pitts wrote: >>> On 3/25/2010 11:06 AM, Eric Sosman wrote: >> >> [... about iterating over a changing List ...] >>>> 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>(); >> >> Not confrontational, just curious: Why prefer a new ArrayList >> to an array? To me, it appears that an ArrayList is just an array >> wrapped up in extra machinery, and I can't see that the machinery >> adds any value for this usage. So, why pay the extra freight? >> What am I missing? > Array is a relatively primitive concept. You're question is akin to > asking why use a "Date object" instead of an "int representing the > milliseconds since Jan 1, 1970". I think you've over-generalized my question. I'm not asking whether arrays or Lists (longs or Dates) are preferable in all circumstances, nor even in most circumstances. Rather, I'm asking why you prefer the "heavier" object in the particular situation Roedy faces. > Either one "works". Yes, the non-primitive has more "machinery", but > that isn't automatically a bad thing. Okay. I'd also say it's not automatically a good thing, especially when the machinery is not going to be used. How much extra machinery are we talking about, in this specific case? I'm suggesting a call to toArray() followed by an iteration over the array: One new object created. A new ArrayList involves creating that same array (perhaps twice, but I don't know whether that happens here; there's a bug number I haven't looked up), creates the ArrayList itself, and creates an Iterator when you actually do the traversal. Three (four?) new objects instead of one. How heavy are the extra objects? The ArrayList carries two fields, plus another inherited from AbstractList. The Iterator carries four more fields (you'll only see three in the source, but inner classes carry a hidden reference to their owner). During the iteration, the Iterator carefully checks whether the ArrayList has been modified; we know it will not have been (the reason we made the snapshot was so no modifications would disturb us). Each item retrieved takes three bounds checks (nominally) instead of one: One in hasNext(), one in next(), and one in the actual array fetch. And so on. Okay, okay, okay: Memory's cheap, CPU's are fast, only misers count their change. But on the other hand, "Take care of the pennies and the pounds will take care of themselves," and "Don't use a cannon to kill a canary." > The primitive looses semantic > meaning outside of its context, where a properly designed abstraction > maintains its semantics regardless of context. The comparison is between an array and an ArrayList. The only semantic difference I see between them is that the latter can grow and shrink, while the former cannot. But in the case at hand, the goal is to get a snapshot that will remain unchanged, that is, to avoid growth and shrinkage. Again, it seems to me we're paying for capabilities that will not be used. > Lists are also easier to work with, and work in more places. Once again, I return to the particular use in consideration. The comparison is between Row[] rows; synchronized(ALL_ROWS) { rows = ALL_ROWS.toArray(new Row[ALL_ROWS.size()]); } for (Row r : rows) { ... } and ArrayList<Row> rows; synchronized(ALL_ROWS) { rows = new ArrayList<Row>(ALL_ROWS); } for (Row r : rows) { ... } There's only one "place" to consider, but still: Point to you for being easier by five keystrokes. (With a simple change I could get a fourteen-key swing and beat you by nine, but at the cost of creating two arrays instead of one.) Even though my first language was FORTRAN, I have no special love for the array nor no special antipathy for the List. I'm happy to use either. But I *do* have a preference for lighter- weight gadgets when they're adequate for the purpose at hand, and a Yankee's aversion to paying for unused extras. YM[*]MV. [*] Motivation. -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
On 3/26/2010 4:18 PM, Daniel Pitts wrote: > [...] > One thing you can do is more the synchronization into the "Row" class, > and turn your operations into atomic operations, rather than have > external clients need to concern themselves with synchronization. > > That way, the Row class can always sync on the same object (probably > "this", but not necessarily), and the client needn't care about it. Are you sure he can do that? He calls three methods on the row instance, in two synchronized blocks. Split them into three blocks, and the pair that were together may no longer see their row in the same state, because something may happen after one method releases the lock and before the next acquires it. -- Eric Sosman esosman@ieee-dot-org.invalid
![]() |
0 |
![]() |
On 26-03-2010 10:22, Eric Sosman wrote: > On 3/25/2010 3:28 PM, Daniel Pitts wrote: >> On 3/25/2010 11:06 AM, Eric Sosman wrote: > >> [... about iterating over a changing List ...] >>> 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>(); > > Not confrontational, just curious: Why prefer a new ArrayList > to an array? To me, it appears that an ArrayList is just an array > wrapped up in extra machinery, and I can't see that the machinery > adds any value for this usage. So, why pay the extra freight? > What am I missing? As a general rule: know that more functionality than array will be needed in the future => pick ArrayList know that more functionality than array will NOT be needed in the future => pick array don't know if more functionality than array will be needed in the future => pick ArrayList My assumption would be that in the real world that would be 10%-10%-80%. Obviously you can argue that this seems fell in category #2. Arne
![]() |
0 |
![]() |
On 3/26/2010 3:19 PM, Eric Sosman wrote: > On 3/26/2010 4:18 PM, Daniel Pitts wrote: >> [...] >> One thing you can do is more the synchronization into the "Row" class, >> and turn your operations into atomic operations, rather than have >> external clients need to concern themselves with synchronization. >> >> That way, the Row class can always sync on the same object (probably >> "this", but not necessarily), and the client needn't care about it. > > Are you sure he can do that? He calls three methods on > the row instance, in two synchronized blocks. Split them into > three blocks, and the pair that were together may no longer see > their row in the same state, because something may happen after > one method releases the lock and before the next acquires it. The first call is synchronized separately. The second and third call are in one synchronized block. The "atomicity" of those two calls can be handled in one method: class MarkedUrl { private final VersionURL url; private final Marker marker; // appropriate contstructor and getters. } public class AppToWatch { public syncronized MarkedUrl getMarkedUrl() { return new MarkedUrl(versionUrl, marker); } } -- Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
![]() |
0 |
![]() |