This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 202814 - IndexOutOfBoundsException: Invalid line index=227 >= lineCount=200
Summary: IndexOutOfBoundsException: Invalid line index=227 >= lineCount=200
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Hints & Annotations (show other bugs)
Version: 7.0
Hardware: All All
: P3 normal (vote)
Assignee: Jan Lahoda
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-29 10:02 UTC by Jesse Glick
Modified: 2012-05-25 14:45 UTC (History)
7 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter: 181479


Attachments
stacktrace (1.33 KB, text/plain)
2011-09-29 10:02 UTC, Jesse Glick
Details
stacktrace (1.42 KB, text/plain)
2011-12-15 07:40 UTC, J Bachorik
Details
stacktrace (1.32 KB, text/plain)
2012-02-20 21:09 UTC, Jesse Glick
Details
stacktrace (1.81 KB, text/plain)
2012-04-24 14:41 UTC, Michel Graciano
Details
stacktrace (1.80 KB, text/plain)
2012-05-17 14:57 UTC, javydreamercsw
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2011-09-29 10:02:39 UTC
This issue was reported manually by mkristofic.
It already has 2 duplicates 


Build: NetBeans IDE Dev (Build 20110919-ae5671919ccf)
VM: Java HotSpot(TM) Client VM, 21.0-b17, Java(TM) SE Runtime Environment, 1.7.0-b147
OS: Linux

Stacktrace: 
java.lang.IndexOutOfBoundsException: Invalid line index=227 >= lineCount=200
   at org.netbeans.modules.editor.lib2.document.LineElementRoot.getElement(LineElementRoot.java:66)
   at org.openide.text.NbDocument$DocumentRenderer.run(NbDocument.java:691)
   at org.netbeans.editor.BaseDocument.render(BaseDocument.java:1404)
   at org.openide.text.NbDocument$DocumentRenderer.renderToInt(NbDocument.java:666)
   at org.openide.text.NbDocument.findLineOffset(NbDocument.java:187)
   at org.netbeans.modules.editor.hints.HintsControllerImpl.computeLineSpan(HintsControllerImpl.java:137)
Comment 1 Jesse Glick 2011-09-29 10:02:43 UTC
Created attachment 111301 [details]
stacktrace
Comment 2 Milutin Kristofic 2011-10-05 15:33:44 UTC
I don't know how to repair it. It looks there is problem in org.openide.text.NbDocument$DocumentRenderer.
Comment 3 J Bachorik 2011-12-15 07:40:39 UTC
Created attachment 114209 [details]
stacktrace

reverting a file outside of IDE
Comment 4 Exceptions Reporter 2011-12-15 07:40:46 UTC
This bug already has 5 duplicates 
see http://statistics.netbeans.org/exceptions/detail.do?id=181479
Comment 5 Jesse Glick 2012-02-20 21:09:57 UTC
Created attachment 115965 [details]
stacktrace

Opened j2ee.ddloaders project.
Comment 6 Michel Graciano 2012-04-24 14:41:26 UTC
Created attachment 118692 [details]
stacktrace

Resolving conflicts
Comment 7 javydreamercsw 2012-05-17 14:57:03 UTC
Created attachment 119578 [details]
stacktrace

Deleting text under a guarded block using Delete key
Comment 8 Miloslav Metelka 2012-05-24 07:41:41 UTC
The Hinter:165 in apisupport:
                errors.add(ErrorDescriptionFactory.createErrorDescription(severity, description, Arrays.asList(fixes), doc, line));

needs to acquire document's readlock e.g. by doc.render() (see javadoc of ErrorDescriptionFactory.createErrorDescription()).
Comment 9 Jesse Glick 2012-05-24 14:49:43 UTC
I was about to fix the Hinter occurrence:

diff --git a/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java b/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
--- a/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
+++ b/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
@@ -153,16 +153,21 @@
          * @param fixes any fixes to offer
          * @see #addStandardAnnotationHint
          */
-        public void addHint(Severity severity, String description, Fix... fixes) {
-            Integer line = null;
+        public void addHint(final Severity severity, final String description, final Fix... fixes) {
+            final Integer line;
             try {
                 lines.run();
                 line = lines.get().get(file.getPath());
             } catch (Exception x) {
                 LOG.log(Level.INFO, null, x);
+                return;
             }
             if (line != null) {
-                errors.add(ErrorDescriptionFactory.createErrorDescription(severity, description, Arrays.asList(fixes), doc, line));
+                doc.render(new Runnable() {
+                    @Override public void run() {
+                        errors.add(ErrorDescriptionFactory.createErrorDescription(severity, description, Arrays.asList(fixes), doc, line));
+                    }
+                });
             } else {
                 LOG.log(Level.WARNING, "no line found for {0}", file);
             }

Then I noticed various other occurrences classified under this bug. If these overloads of createErrorDescription require a render lock, why does ErrorDescriptionFactory not just acquire it automatically, since apparently even members of the editor team forget to do this, e.g. jlahoda in RunFindBugs? Better yet, do it in NbDocument, which would help other occurrences such as merge.builtin.visualizer.MergePanel.showLine12.

Then I looked again at the stack traces and saw that NbDocument.findLineOffset already _does_ acquire a render lock. And it is documented to throw IndexOutOfBoundsException, which computeLineSpan fails to catch (or rethrow as BadLocationException). Note further that fullLine does catch BadLocationException, but (a) reports it using deprecated ErrorManager, (b) returns an undocumented null which if passed to the ErrorDescription ctor will later result in NPEs.

Clearly you need to document and test the behavior of createErrorDescription w.r.t. invalid line numbers. What is actually causing it to be called with an invalid line number is another question; merely wrapping this call in a document render lock is unlikely to help anything if the line number was computed elsewhere.
Comment 10 Jan Lahoda 2012-05-24 20:24:01 UTC
(In reply to comment #9)
> I was about to fix the Hinter occurrence:
> 
> diff --git
> a/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
> b/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
> ---
> a/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
> +++
> b/apisupport.refactoring/src/org/netbeans/modules/apisupport/hints/Hinter.java
> @@ -153,16 +153,21 @@
>           * @param fixes any fixes to offer
>           * @see #addStandardAnnotationHint
>           */
> -        public void addHint(Severity severity, String description, Fix...
> fixes) {
> -            Integer line = null;
> +        public void addHint(final Severity severity, final String description,
> final Fix... fixes) {
> +            final Integer line;
>              try {
>                  lines.run();
>                  line = lines.get().get(file.getPath());
>              } catch (Exception x) {
>                  LOG.log(Level.INFO, null, x);
> +                return;
>              }
>              if (line != null) {
> -               
> errors.add(ErrorDescriptionFactory.createErrorDescription(severity,
> description, Arrays.asList(fixes), doc, line));
> +                doc.render(new Runnable() {
> +                    @Override public void run() {
> +                       
> errors.add(ErrorDescriptionFactory.createErrorDescription(severity,
> description, Arrays.asList(fixes), doc, line));
> +                    }
> +                });
>              } else {
>                  LOG.log(Level.WARNING, "no line found for {0}", file);
>              }
> 
> Then I noticed various other occurrences classified under this bug. If these
> overloads of createErrorDescription require a render lock, why does
> ErrorDescriptionFactory not just acquire it automatically, since apparently

The point of taking the read lock before calling the method is to be able to check if the document was changed since the warning computation started, and stop and restart the computation if it was (so that the warnings are not shown on proper places). (There are other ways to do that, of course. But from a quick look at LayerHints, it seems that the code will happily place warnings on incorrect lines without any attempt to correct that if a document modification happens at the correct moment.)

> even members of the editor team forget to do this, e.g. jlahoda in RunFindBugs?

I am not going to comment on this one.

> Better yet, do it in NbDocument, which would help other occurrences such as
> merge.builtin.visualizer.MergePanel.showLine12.

NbDocument.findLineOffset takes the read lock, of course (through the DocumentRenderer). No idea how and why that should help the MergePanel and similar cases.

> 
> Then I looked again at the stack traces and saw that NbDocument.findLineOffset
> already _does_ acquire a render lock. And it is documented to throw
> IndexOutOfBoundsException, which computeLineSpan fails to catch (or rethrow as
> BadLocationException). Note further that fullLine does catch
> BadLocationException, but (a) reports it using deprecated ErrorManager, (b)
> returns an undocumented null which if passed to the ErrorDescription ctor will
> later result in NPEs.
> 
> Clearly you need to document and test the behavior of createErrorDescription
> w.r.t. invalid line numbers. What is actually causing it to be called with an

There are generally two ways to resolve this request:
-never throw the IOOBE from the method. Use last document's line when a the given line number is not in the document. Would make it more consistent with the other methods. Would definitely be easier for me (as the FindBugs through Source/Inspect... may sometimes inevitably run into this, and I also wouldn't need to have this discussion again). Would make it more difficult to find out that the client does something wrong (as do FB through editor and the Hinter).
-leave it more or less as it is (possibly throw the IOOBE from the API method, so that the exception reporter splits the stack traces correctly). Basically opposite dis-/advantages.

> invalid line number is another question; merely wrapping this call in a
> document render lock is unlikely to help anything if the line number was
> computed elsewhere.

True, the client must do a bit more to behave sanely - not quite sure why this was assigned to the infrastructure then. Nothing I can do in spi.editor.hints can fix the Hinter (or FindBugs).
Comment 11 Jesse Glick 2012-05-24 22:09:44 UTC
(In reply to comment #10)
> from a quick look at LayerHints, it seems that the code will happily place
> warnings on incorrect lines without any attempt to correct that if a
> document modification happens at the correct moment.

Likely true; LayerHints is designed to only run on unmodified documents (since the layer parser currently loads from disk only), so it makes no attempt to adjust to last-minute edits. It is not a big deal if the line number for a hint is momentarily off by one, or indeed if the hint is suppressed in this case, so long as the IDE does not throw exceptions as a result.

> No idea how and why that should help the MergePanel and similar cases.

I do not think it would either; just responding to mmetelka's comment #8 which implies that acquiring the render lock suffices to prevent the IOOBE.

> never throw the IOOBE from the method. Use last document's line when a the
> given line number is not in the document.

This would get my vote. (Or set the line number to -1 and make sure that the hint is simply not displayed in this case.) If it would be much more work to prevent the race condition in every case, and it is not a heavily used hint, it is probably not worth the effort of correcting (and the risk of introducing a deadlock or similar).

> Would make it more difficult to find out that the client does something wrong

Could issue a (non-stack) warning in console with some diagnostic information. Might or might not be noticed by the hint developer of course.
Comment 12 Jan Lahoda 2012-05-25 14:22:18 UTC
(In reply to comment #11)
> > No idea how and why that should help the MergePanel and similar cases.
> 
> I do not think it would either; just responding to mmetelka's comment #8 which
> implies that acquiring the render lock suffices to prevent the IOOBE.

That was (a somewhat shorter version of) my comment through Mila. The point is that the callers need to take into account possible changes to the document between the point the snapshot from which the warnings are produced has been taken, and the current point.

> 
> > never throw the IOOBE from the method. Use last document's line when a the
> > given line number is not in the document.
> 
> This would get my vote. (Or set the line number to -1 and make sure that the
> hint is simply not displayed in this case.) If it would be much more work to
> prevent the race condition in every case, and it is not a heavily used hint, it
> is probably not worth the effort of correcting (and the risk of introducing a
> deadlock or similar).

FindBugs:
http://hg.netbeans.org/jet-main/rev/a02b2d1917a9
spi.editor.hints:
http://hg.netbeans.org/jet-main/rev/47552805892f
PHP:
http://statistics.netbeans.org/exceptions/detail.do?id=188389
Diff:
http://statistics.netbeans.org/exceptions/detail.do?id=188390
Comment 13 Miloslav Metelka 2012-05-25 14:45:47 UTC
Good, thanks.
I'm just wondering whether we could more generically associate gathered line-related information with the document versions and possibly compute diffs (similarly like LineSet.getOriginal()) but that would be a long term thing.