Bug 32436

The close button showed up barely on the options panel, and after I
pushed it, the IDE came back under my control.

Created attachment 9580 [details]
log file

Just a note: the bug donoyeB is speaking about is probably 
issue #31371, closed as WORKSFORME.

Yes, it is exactly the same issue.  Why have you closed it in exactly
the same manner?  It was not resolved then, and its rearing its ugly
head again.

This is clearly a bug that needs work.  I was working on a 1 week old
install of win2K as well as a fresh ~1week install of NB.

I am going to fix it this time around.

Tried the 26th build, didn't do it.
Tried the 27th build, it did.

Perhaps this is because the 27th build is colapsing the 
tree again, like the ide used to do long ago!?

Would someone care to comment on the colapsing tree/table!?

No question, this is the same issue.  Just open and close 
options ~3 times, and bam! it should be _easy_ to find 
given its repeatability.

Going to P1.  This renders the IDE Generally unusable.  You 
can reset to p2 if you feel you should, but I think 
unusable IDE is p1 right?

If you mouseover the options close button location, it will 
show up.  Then I closed the options window, and noticed the 
memory bar moving around in NB.  This means its a race and 
not a starvation.  Change title.

Created attachment 9589 [details]
dump

*** Issue 31371 has been marked as a duplicate of this issue. ***

New info.
Conditions to repeat.
windows2000
jdk 1.3.1_07

All other windows must be closed.  Just a normal ide with 
nothign open.  What I mean is just close all source windows.

filesystem tree or projects tree, whichever is showing must 
be totally collapsed.

compiler option must be showing.  In other words, when you 
open options everything is closed.  First open 'Building' 
node, then open 'Compiler Types' node.

Now you hit the close button.
Then you open options window again.
Then you hit close button.
Then you open options again, etc.

Should take about 2-3 times before it starts racing.

org.netbeans.core.windows.frames.TopFrameTypeImpl line 187 
which is

SwingUtilities.invokeLater(this);

is done over and over and over, etc...Its an infinite loop, 
but I guess its lax since its invoke later, so you can 
sneak out if your CPU is fast enough I guess.

Anyway, for some reason my ide will NOT let me do a debug 
session with 1.3.1, so I been using my laptop.  What a pain 
that is...Be glad when I fully switch to linux.

Anyway, can I get some help on this now?  If I could debug 
I could track down why its looping, but debugging across 
network is too painful.

Here is the offending method.  Note that it contains 
recursive calls on the anonymous class defined within.

private void tryRequestFocus() {
    if(SwingUtilities.findFocusOwner(this) != null) { // 
From jdk1.4 replace with isFocused().
        return;
    }

    TopFrameTypeImpl pendingFocusRequestor
        = (TopFrameTypeImpl)pendingFocusRequestorRef.get();
    if(pendingFocusRequestor == this) {
        if(this != selectedFrameRef.get()) { // From jdk1.4 
replace with !isActive().
            requestFocus();
        }
    } else if(pendingFocusRequestor == null) {
        pendingFocusRequestorRef = new WeakReference(this);
        requestFocus();
    } else {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                TopFrameTypeImpl pendingFocusRequestor
                    = (TopFrameTypeImpl)
pendingFocusRequestorRef.get();
                // Validate the pending focus requestor.
                if(pendingFocusRequestor != null
                && ((pendingFocusRequestor == 
selectedFrameRef.get()) // From jdk1.4 replace with isActive
().
                == (SwingUtilities.findFocusOwner
(pendingFocusRequestor) != null))) { // From jdk1.4 replace 
with isFocused().
                    pendingFocusRequestorRef = new 
WeakReference(null);
                }

                // If there is no pending focus request, 
try to request focus.
                if(pendingFocusRequestor == null) {
                    // OK, make another attempt.
                        
TopFrameTypeImpl.this.tryRequestFocus();
                } else {
                    // Focus request is still in progress.
                    System.out.println("Doing 
SwingUtilities.invokeLater(this); for TopFrameTypeImpl");
                    Thread.dumpStack();
                    try{
                        Thread.sleep(500);
                    }
                    catch(Exception e){}
                    SwingUtilities.invokeLater(this);
                }
            }
        });
    }
}

I think the 2nd invokeLater is just unnecessary.  Removing 
it does seem to fix the issue, but I have no idea why it 
was their in the first place so I need someone to double 
check on that.

My first test, I put a little delay in it so that my log 
wouldnt bust.  Then it occurs to me that perhaps a little 
delay belongs in it.  Basically if the focus request does 
not work, put it back into the queue.  But not instantly 
because if its instant, the ide can start looping.  I mean 
if it cant do the focus, it NEEDS to wait because something 
else must change before it tries again.

So maybe the correct action is to put a delay?

anyway, there it is.

Note: the method I cut and pasted above contains things I 
added which are

System.out.println("Doing SwingUtilities.invokeLater(this); 
for TopFrameTypeImpl");
Thread.dumpStack();
try{
  Thread.sleep(500);
}
catch(Exception e){}

current method does not have these.

I will investigate

Hmm it seems you close Options window before it gets focus and code
does not handle it correctly - in such case loop should be finished. I
tried to reproduce it but I cannot. I attach patch (put it to
lib/patches) please could you test it? Thanks.

Created attachment 9602 [details]
Patch

Patch didn't fix it.  This is what I got just before I was able to get
the options panel to close

...
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
POST REQUEST FOCUS
BREAK LOOP FRAME IS CLOSED

When I got it to close, I got the last line, before that the  output
was just looping the POST REQUEST FOCUS message.

Reproduction is not automatic, it takes a few tries.  And sometimes
the compile node is not required to be open.  I had to open and close
about 5-10 times in succession before I got the bug to happen. 
Sometimes only 2-3 times, sometimes more.

To me it seems patch works: Last message means that frame is not
showing (ie. was closed) so loop is canceled.

Not exactly :D

It was closed because I closed it.  If you read all the messages in
this bug report, you will notice a few times where I say that it is
not complete starvation and it is not completely locked up.  This is
the case.  But make no mistake, 

1. the console was filled with "POST REQUEST FOCUS" and the ide was
looping.
2. The IDE was not properly redrawing and I could not use any aspects
of it.  It was live locked.
3. You can stop this by guessing where the 'close' button is on the
options panel (because it is not displaying properly).  If you guess
correct (easy because the button is in the same place) then you can
get the options panel to close after a few seconds.

My ability to get the options panel to close does not mean the error
is not there.  If I did not close the options panel, the ide would be
in an unusable state.  Again, the IDE is not redrawing itself,
everything is unusable, and I am only lucky to be able to close the
options panel.  Have you tried to reproduce?  Its not that hard to do
if you follow instructions carefully.  I can get it on both of my
computers in less than 30 seconds.  1 is ~300MHz PIII, other is XP2100+.

Did you look at the code I cut and pasted?  Do you understand that
code must change!?  Even if you fix the inspiration for the loop, that
code is still flawed.  AT the very least it should include a
Thread.yield();  But for me it needs a delay.  Unfortunately any delay
will be an arbitrary value and to me that shows something
fundamentally wrong with the approach or the technique.

It seems focus is not assigned to Options window for some reason. We
added timeout so when focus is not assigned during some time to any
component inside frame we will cancel loop.

To your last comment: This code is performed in AWT thread!! So we
cannot use sleep() and yield() will not help.

Problems seems that we request focus and wait till focus is not in
selected frame which is not the case. (And generaly we cannot expect
that focus will be assigned to any component inside frame eg. there
can be nonfocusable component.)

Is tryRequestFocus single threaded?

pendingFocusRequestorRef seems to be getting used by multiple threads,
but their is no synchronization.

Same with selectedFrameRef its used without synchronization.  I am
assuming that setSelected must only be called from AWT thread!?

This cant be because of other things I am seeing in the code.

Wish I could understand what this is supposed to do, but it does seem
to be missing a little synchronization.

OK, so its always in the AWT thread.  That settles part of it.

line 178 

pendingFocusRequestorRef = new WeakReference(null);

Why isint that followed by

pendingFocusRequestor = null;

It should be, and that would let line 182 execute.  Is that right?

Created attachment 9610 [details]
New patch

Sorry I am not able to undertand code now (too late evening - I am
going home - too smart tricky code). Peter please take care about and
explain it better in code.

Hey man, get a cup of coffee, and code on! :D

It is rather complicated.

I think the problem is because of a improper clearing of
pendingFocusRequest and pendingFocusRequestRef variables.  But i'm
trying to figure out where and why.

The whole tryRequestFocus method seems suspect.  Certainly the if
construct on line 175 is bizarre.  It seems to have too many '=='. 
What exactly is the problem and what are we solving with this method?

If the focused component is closed, then the next component will
receive focus based on the transferFocus() command.  This does not
update the selectedFrameRef variable.  This seems awkward.

that variable likely should be set as a result of a focus listener?

Vhat is the status now?

To the entire hack. Well, I admit it is too tricky, was written too
quickly (that mentioned '==' is just typo, there should be '&&' I guess).
 I hope we get rid of it in new winsys, but for the moment it seems it
is has to be there.
I also think the fix of this isn't complicated.

The problem seems to be there was requested focus, but it doesn't
come, what leads to the neverending wait loop. Therefore I suggested
to put there some timeout. Yes I know, it will be even more
complicated, but.. .
Marek, is it that the last patch.
dnoyeB, does it work?

dnoyeB said that second patch works. dnoyeB please confirm.

I looked deep into the code and concluded...hack it :D


Marek's patch works, but its not very nice.  Lets try and 
fix that '==' first and see what happens.  I think the main 
error in this 'hack' is at that point because its not 
clearing the pendingFocusRequestorRef because of that typo.

Also, once the ref is cleared it makes sense to clear the 
pendingFocusRequestor, itself.  that will allow the thing 
to clean itself up quickly.

Lets try that first before we try the patch Marek put 
together.

Last patch _2 contains fixed typo '==' is replaced by '&&'.

Created attachment 9625 [details]
Patch

patch verified

I am able to reproduce it on my machine with Linux with dev build as
described. (JDK 1.3.1_07, JDK 1.4.1_02) Not with release35 build. We
decided to commit this fix also ro release35 branch because it can
happen there too. I checked and it is caused by not receiving focus
inside selected frame -
SwingUtilities.findFocusOwner(pendingFocusRequestor) returns always
null in such case and it causes infinite loop. Fix adds timeout so
loop ends even if focus is not transfered inside selected frame.

Created attachment 9630 [details]
Patch without debugging logs

Created attachment 9631 [details]
Diff

I reviewed the fix. It seems it is safe. I vote for integration.

Peter, the focus never gets the chance to come to the original
requestor because of the racing done by the tryRequestFocus() method
on behalf of the 2nd requestor (unsure if this is same object or not).
 If you delay the recursion within that method, then it only loops a
few times, and the focus is gained.  Even if the delay is in the AWT
thread.

Marek, its not in latest dev builds because in order for this to
happen you need the options window to open with the tree collapsed. 
But opening with the tree collapsed was appearantly a regression and a
mistake, so when that is fixed, the options pane wont exhibit this
behavior.  But of course their is something buggy here.  

new winsys?  I hope that cleans it up as well.  I am struggling to
understand why NB uses practically its own windowing system when I
though that task would be handled by Swing.  I am sure their is a good
reason, but I don't see it yet.

Testing patches.

The fix works.  We need a resolved->Hacked option :D

It seems as if the whole methodology is broken there.  Whenever the
situation comes about that the method was supposed to fix, thats when
the whole thing fails.  And it also seems as if once you get the
tryRequestFocus method to loop once, everytime the method is called
after that, it will loop.  For instance, at first my log was empty. 
Then once I triggered the event, the patch did handle it.  but after
that, each time I opened the options panel, messages were dumped to
the log indicating that each opening after that point was causing this
loop.

Enough is enough I suppose.  Can't teach an old dog new tricks and all
of that ;)  Write it off and hope the next system implementation will
alleviate this.

its a go!

fix approved by release coordinator for 3.5.

Thanks all for nailing this down!

Fix is comitted. Timeout added to avoid infinite loop when focus is
not gained by any component inside frame.

Modified (trunk):
core/src/org/netbeans/core/windows/frames/TopFrameTypeImpl.java r.1.27

relese35:
core/src/org/netbeans/core/windows/frames/TopFrameTypeImpl.java r.1.26.2.1

dnoyeB,

I hope it's already fixed, if not please reopen, thanks for cooperation.

verified