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: Fabio Z. <fa...@gm...> - 2008-08-22 11:06:17
|
Hi Radim... Just fixed it. Thanks for reviewing it!
Cheers,
Fabio
On Fri, Aug 22, 2008 at 2:52 AM, Radim Kubacki <ra...@ku...> wrote:
> 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
> >
>
|
