Bug 32747

Summary: Temporary deadlock printing a stack trace to output window
Product: cnd Reporter: Jesse Glick <jglick>
Component: TerminalemulatorAssignee: _ tboudreau <tboudreau>
Status: VERIFIED FIXED QA Contact: ivan <ivan>
Priority: P1 CC: mmirilovic
Version: 3.xKeywords: THREAD
Target Milestone: 6.x   
Hardware: PC   
OS: Linux   
Whiteboard:
Issue Type: DEFECT Exception Report:
Attachments: Thread dump
Test case - save in sampledir, run w/ internal execution -> hang for a minute or so
Resulting thread dump (r35)
Suggested patch (r35 or trunk - tested in both)
Binary patch - place in $nbhome/modules/autoload/patches/org-netbeans-core-output/32747.jar

Description Jesse Glick 2003-04-09 02:31:46 UTC
[dev 030402]

See attached thread dump. Was just printing to an
OW output stream from an action.

Seemed to clear itself up after a moment.

Problem:
OutputTabTerm.OutTermPane.TermOutputWriter is
improperly synchronized. Throwable.printStackTrace
synchronizes on a PrintWriter, so you cannot post
a request into the AWT thread which also tries to
synch on it.
Comment 1 Jesse Glick 2003-04-09 02:32:16 UTC
Created attachment 9790 [details]
Thread dump
Comment 2 Jesse Glick 2003-04-09 02:41:14 UTC
Reproducible, by the way.

Workaround is to change the lock:

PrintWriter err = IOProvider.getDefault().getIO("Errors", false).getErr();
err = new PrintWriter(err) {
    {
        lock = new Object();
    }
};
exc.printStackTrace(err);
Comment 3 _ tboudreau 2003-04-09 10:16:52 UTC
Jesse, since you are able to reproduce the problem and have
a fix, I'm reassigning to you.  If you feel
strongly that I should fix it, let me know more clearly
how to reproduce it (when to do the stack dump, etc.).
Comment 4 Jesse Glick 2003-04-09 17:44:37 UTC
Can be easily reproduced. Will attach test class and thread dump found
in r35.
Comment 5 Jesse Glick 2003-04-09 17:46:38 UTC
Created attachment 9820 [details]
Test case - save in sampledir, run w/ internal execution -> hang for a minute or so
Comment 6 Jesse Glick 2003-04-09 17:47:16 UTC
Created attachment 9821 [details]
Resulting thread dump (r35)
Comment 7 Jesse Glick 2003-04-09 17:57:46 UTC
The temporary aspect of the hang is probably because
OutputTabTerm.OutTermPane.TermOutputWriter.invokeNow only waits for
500msec per invocation, though I don't understand the precise mechanics.

The deadlock arises because:

1. Throwable.printStackTrace locks on the PrintWriter it is printing
to. This is not exactly documented but does not seem unreasonable.

2. That PrintWriter is a TermOutputWriter. In the caller's thread,
with a monitor held, it calls invokeNow on the AWT thread.

3. In the AWT thread, appendText is called, which is synchronized.
That tries to acquire the monitor and fails.

4. When the wait times out, I suppose the monitor will be released in
caller's thread, and then the appendText call succeeds.

Possible fix:

1. Do not make appendText methods synchronized on this. Use some other
unique object to synch on.

2. In TermOutputWriter's constructor, set the protected variable lock
(from Writer) to some new Object(). I am not precisely sure why this
works, but I think it does, based on the fact that this workaround works:

// ...as before
PrintWriter w2 = new PrintWriter(w) {
    {
        lock = new Object();
    }
};
for (/* as before... */) {
    new Throwable(msg.toString()).printStackTrace(w2);
}
// as before...
Comment 8 Jesse Glick 2003-04-09 17:59:22 UTC
Sorry, probably suggested fix #2 is nonsense - only helped because
then Throwable.printStackTrace was synching on a different PrintWriter.

So recommend fix #1 - change synch of appendText method to use some
private lock.
Comment 9 Jesse Glick 2003-04-09 18:02:19 UTC
Well, maybe not. Just wrapping w in w2 = new PrintWriter(w) does not
work. You have to set a new lock for that workaround to take effect.
Comment 10 Jesse Glick 2003-04-09 18:05:11 UTC
...because PrintWriter.println synchs on the lock (not this), where
the lock is by default the delegated-to writer. So the correct
workaround has to ensure that nobody synchs on the TermOutputWriter -
neither T.pst nor PW.pln. So yes, fix #2 is not going to work.
Comment 11 Jesse Glick 2003-04-09 18:29:09 UTC
Created attachment 9824 [details]
Suggested patch (r35 or trunk - tested in both)
Comment 12 Jesse Glick 2003-04-09 18:29:54 UTC
Created attachment 9825 [details]
Binary patch - place in $nbhome/modules/autoload/patches/org-netbeans-core-output/32747.jar
Comment 13 Jesse Glick 2003-04-09 18:31:06 UTC
I am having difficulty getting the binary patch to work for some
reason. However the source patch does appear to work in r35 and trunk
both: compile and run Test32747.java w/ Internal Execution; without
patch, IDE will hang for a couple minutes; with patch, it will show
all the stack traces at once.
Comment 14 pzajac 2003-04-10 08:23:23 UTC
Jesse I have tested your patch in nb35 on linux and I haven't found
any problem.  Your testcase work correctly with patch. No deadlock
occured.  
Comment 15 Jesse Glick 2003-04-10 16:09:09 UTC
To help QA determine priority:

1. Bug causes temporary deadlock, proportional to # of stack traces.
No data loss if user knows to wait a couple of minutes or so and does
not immediately kill process.

2. Only known to occur when printing a stack trace using *internal*
execution. Running an external process which prints a stack trace does
not matter - the lines are sent one by one to the Output Window and it
displays them. Only if something running inside NB calls
Throwable.printStackTrace on an OutputWriter (API to the Output
Window) will the deadlock be possible. I am not sure if any code in
the regular NB distribution does this (other than Internal Execution
on Java classes); it is possible in other modules not in the distribution.

3. Not every stack trace seems to trigger the deadlock. Seems to need
to be repeated at least once before the deadlock will happen. The test
case I attached meets this condition. E.g. smaller test case:

OutputWriter w = IOProvider.getDefault().getIO("#32747", false).getErr();
for (int c = 0; c < 5; c++) {
    new Throwable().printStackTrace(w);
}
Comment 16 Marian Mirilovic 2003-04-10 17:28:19 UTC
Thanks for clarification Jesse.

I am agree with integration the fix into 3.5 . 

I hope patch is verified by Peter.
Comment 17 Jan Chalupa 2003-04-11 16:31:34 UTC
P2 -> P1. This should go into release35.
Comment 18 _ tboudreau 2003-04-14 13:10:57 UTC
Fixed in trunk, awaiting approval for integration to 3.5 branch.

Checking in core/output/src/org/netbeans/core/output/OutputTabTerm.java;
/cvs/core/output/src/org/netbeans/core/output/OutputTabTerm.java,v 
<--  OutputTabTerm.java
new revision: 1.6; previous revision: 1.5
Comment 19 _ ttran 2003-04-14 13:24:56 UTC
approved for 3.5
Comment 20 _ tboudreau 2003-04-14 14:37:49 UTC
Fixed for release35 branch

Checking in OutputTabTerm.java;
/cvs/core/output/src/org/netbeans/core/output/OutputTabTerm.java,v 
<--  OutputTabTerm.java
new revision: 1.5.2.1; previous revision: 1.5
done
Comment 21 Marian Mirilovic 2003-06-12 10:50:55 UTC
verified
Comment 22 Quality Engineering 2008-12-23 08:39:33 UTC
moving terminal emulator issues to terminalemulator component.
To see the correct version and target milestone of this issue look at Issue
Activity table.
By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo