Re: [Pydev-cvs] org.python.pydev/src/org/python/pydev/outline PyOutlinePage.java, 1.25, 1.26 Outli
Brought to you by:
fabioz
Menu
▾
▴
Re: [Pydev-cvs] org.python.pydev/src/org/python/pydev/outline PyOutlinePage.java, 1.25, 1.26 OutlineLinkWithEditorAction.java, 1.8, 1.9
|
From: Radim K. <ra...@ku...> - 2008-08-22 05:52:26
|
A comment about your synchronization patch:
When I saw it I thought it cannot work. But I was surprised that
autoboxing is so smart that
Integer i; i++;
causes conversion from Integer to int, increment and then (even if it
is postfix increment) conversion back to Integer and storing to i
variable. So far so good.
2nd idea: why volatile when every access is done inside this
synchronized block?
And now there is a reason why it does not work as you probably expect:
Integer is an immutable object after each ++/-- you have another
instance to synchronize on. So there can be more threads inside this
synchronized block than one. For example if linkLevel is 0 one thread
can start relinkAll and increment it, second thread calls unlinkAll
and executes remove*() before add*() is called in 1st thread because
it now sees different object assgned to linkLevel (and it volatile to
inrease the chance that it will happen).
Use
private final Object lock = new Object();
private int linkLevel = 1
+ synchronized(lock)
Without the need for 1.4 compatibility you could also use
AtomicInteger.{increment|decrement}AndGet()
Yet another reason why your patch is bad: if you read
http://java.sun.com/docs/books/jls/third_edition/html/conversions.html#5.1.7
carefully you will see that the intent is to provide the same instance
for the same primitive values. Synchronizing on a publicly known
object is wrong. If someone else starts to do the same at some other
random place in the codebase you can get interesting contention
(better) or deadlocks (worse).
-Radim
On Thu, Aug 21, 2008 at 1:56 PM, Fabio Zadrozny
<fa...@us...> wrote:
> Update of /cvsroot/pydev/org.python.pydev/src/org/python/pydev/outline
> In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv19176/src/org/python/pydev/outline
>
> Modified Files:
> PyOutlinePage.java OutlineLinkWithEditorAction.java
> Log Message:
> Minor changes for console / possible race condition on linking with outline page.
>
> Index: PyOutlinePage.java
> ===================================================================
> RCS file: /cvsroot/pydev/org.python.pydev/src/org/python/pydev/outline/PyOutlinePage.java,v
> retrieving revision 1.25
> retrieving revision 1.26
> ***************
> *** 299,313 ****
> }
>
> void unlinkAll() {
> ! removeSelectionChangedListener(selectionListener);
> ! if(linkWithEditor != null){
> ! linkWithEditor.unlink();
> }
> }
>
> void relinkAll() {
> ! addSelectionChangedListener(selectionListener);
> ! if(linkWithEditor != null){
> ! linkWithEditor.relink();
> }
> }
> --- 305,343 ----
> }
>
> + /**
> + * Used to hold a link level to know when it should be unlinked or relinked, as calls can be 'cascaded'
> + */
> + private volatile Integer linkLevel = 1;
> +
> + /**
> + * Stops listening to changes (the linkLevel is used so that multiple unlinks can be called and later
> + * multiple relinks should be used)
> + */
> void unlinkAll() {
> ! synchronized (linkLevel) {
> ! linkLevel--;
> ! if(linkLevel == 0){
> ! removeSelectionChangedListener(selectionListener);
> ! if(linkWithEditor != null){
> ! linkWithEditor.unlink();
> ! }
> ! }
> }
> }
>
> + /**
> + * Starts listening to changes again if the number of relinks matches the number of unlinks
> + */
> void relinkAll() {
> ! synchronized (linkLevel) {
> ! linkLevel++;
> ! if(linkLevel == 1){
> ! addSelectionChangedListener(selectionListener);
> ! if(linkWithEditor != null){
> ! linkWithEditor.relink();
> ! }
> ! }else if(linkLevel > 1){
> ! throw new RuntimeException("Error: relinking without unlinking 1st");
> ! }
> }
> }
>
> Index: OutlineLinkWithEditorAction.java
> ===================================================================
> RCS file: /cvsroot/pydev/org.python.pydev/src/org/python/pydev/outline/OutlineLinkWithEditorAction.java,v
> retrieving revision 1.8
> retrieving revision 1.9
> diff -C2 -d -r1.8 -r1.9
> *** OutlineLinkWithEditorAction.java 5 Feb 2008 23:11:12 -0000 1.8
> --- OutlineLinkWithEditorAction.java 21 Aug 2008 20:56:28 -0000 1.9
> ***************
> *** 25,28 ****
> --- 25,34 ----
> * editor.
> *
> + * Design notes:
> + * It's linked on the constructor and unlinked in the destructor.
> + *
> + * It considers that it's always linked, even if the action is inactive, but before executing it, a
> + * check is done to see if it's active or not.
> + *
> * @author Fabio
> */
> ***************
> *** 40,43 ****
> --- 46,52 ----
> }
>
> + /**
> + * When called, it STOPS hearing notifications to update the outline when the cursor changes positions.
> + */
> public void unlink() {
> PyEdit edit = pyEdit.get();
> ***************
> *** 47,50 ****
> --- 56,62 ----
> }
>
> + /**
> + * When called, it STARTS hearing notifications to update the outline when the cursor changes positions.
> + */
> public void relink() {
> PyEdit edit = pyEdit.get();
> ***************
> *** 158,161 ****
> --- 170,176 ----
>
>
> + /**
> + * @return The parsed item that should be selected given the startLine passed.
> + */
> private ParsedItem findSel(ParsedItem r, int startLine) {
> ParsedItem prev = null;
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Pydev-cvs mailing list
> Pyd...@li...
> https://lists.sourceforge.net/lists/listinfo/pydev-cvs
>
|
