This is quite an extensive patch, I am surprised so much work was necessary to make this possible. I am not familiar enough with the code to review this myself so I am assigning it to Matthieu.
However, before anyone can test it, it seems you missed a file in your patch: unable to find IndexInterruptedException symbol. Patch applies cleanly but plugin no longer builds without errors.
Last edit: Alan Ezust 2014-12-28
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Looking some more, I am thinking this is probably not the right approach. You are directly cancelling Threads, when the cancel of an item from the jEdit Task Monitor can cause Task.cancel() to be called, and from there, cleanly exit the long-running task...
I think the thread management code is supposed to be centralized, so we can work with Tasks as a layer between us and the underlying threads.
But again, I am not familiar enough with the TaskManager code either. Matthieu, what do you think?
It should not be necessary to introduce a new Exception type for something like this, is it?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Under the covers the Task.cancel() just calls interrupt() on the thread that is running the task.
The original problem was caused when the thread was interrupted using Task.cancel(), it is running the indexing in the Lucene code.
Lucene uses the java.nio package to create the index files, and when this is interrupted it throws a ClosedByInterruptException. As this is a sub class IOException, then it gets caught by the existing catch block for IOException, which just logs the error and carries on processing, hence the task does not cancel.
java.nio.channels.ClosedByInterruptException
at java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202)
at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:919)
at org.apache.lucene.store.MMapDirectory.map(MMapDirectory.java:283)
at org.apache.lucene.store.MMapDirectory$MMapIndexInput.<init>(MMapDirectory.java:228)
at org.apache.lucene.store.MMapDirectory.openInput(MMapDirectory.java:195)
at org.apache.lucene.index.SegmentInfos.read(SegmentInfos.java:285)
at org.apache.lucene.index.SegmentInfos$1.doBody(SegmentInfos.java:347)
at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:783)
at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:630)
at org.apache.lucene.index.SegmentInfos.read(SegmentInfos.java:343)
at org.apache.lucene.index.StandardDirectoryReader.isCurrent(StandardDirectoryReader.java:326)
at org.apache.lucene.index.StandardDirectoryReader.doOpenNoWriter(StandardDirectoryReader.java:284)
at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:247)
at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:235)
at org.apache.lucene.index.DirectoryReader.openIfChanged(DirectoryReader.java:170)
at gatchan.jedit.lucene.AbstractIndex.initReader(AbstractIndex.java:141)
at gatchan.jedit.lucene.AbstractIndex.getSearcher(AbstractIndex.java:119)
at gatchan.jedit.lucene.CentralIndex.addFile(CentralIndex.java:128)
at gatchan.jedit.lucene.IndexImpl.addDocument(IndexImpl.java:385)
at gatchan.jedit.lucene.IndexImpl.addFiles(IndexImpl.java:151)
at gatchan.jedit.lucene.LucenePlugin.addToIndex(LucenePlugin.java:344)
at gatchan.jedit.lucene.IndexProjectAction$ProjectIndexer._run(IndexProjectAction.java:87)
at org.gjt.sp.util.Task.run(Task.java:64)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:744)
The solution to the problem is to catch the ClosedByInterruptException thrown by the Lucene library and terminate the thread gracefully.
I tried calling Thread.currentThread().interrupt(); at the point where the ClosedByInterruptException was thrown and that didn't work, so to close the thread gracefully I threw a new IndexInterruptedException to cleanly get back to the top level and exit the thread gracefully.
Most of the changes are just adding throws clauses to propogate the up the call stack, and by running the same check for the other calls to the Lucene library, so the changes are rather more extensive than I originally envisaged.
It looks like svn diff doesn't pick up new files so included is the new file.
sorry that should have said "caught by the existing catch block for IOException, which just logs the error and carries on processing, hence the task does not cancel."
amended post
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This is quite an extensive patch, I am surprised so much work was necessary to make this possible. I am not familiar enough with the code to review this myself so I am assigning it to Matthieu.
However, before anyone can test it, it seems you missed a file in your patch: unable to find IndexInterruptedException symbol. Patch applies cleanly but plugin no longer builds without errors.
Last edit: Alan Ezust 2014-12-28
Looking some more, I am thinking this is probably not the right approach. You are directly cancelling Threads, when the cancel of an item from the jEdit Task Monitor can cause Task.cancel() to be called, and from there, cleanly exit the long-running task...
I think the thread management code is supposed to be centralized, so we can work with Tasks as a layer between us and the underlying threads.
But again, I am not familiar enough with the TaskManager code either. Matthieu, what do you think?
It should not be necessary to introduce a new Exception type for something like this, is it?
Under the covers the Task.cancel() just calls interrupt() on the thread that is running the task.
The original problem was caused when the thread was interrupted using Task.cancel(), it is running the indexing in the Lucene code.
Lucene uses the java.nio package to create the index files, and when this is interrupted it throws a ClosedByInterruptException. As this is a sub class IOException, then it gets caught by the existing catch block for IOException, which just logs the error and carries on processing, hence the task does not cancel.
java.nio.channels.ClosedByInterruptException
at java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202)
at sun.nio.ch.FileChannelImpl.map(FileChannelImpl.java:919)
at org.apache.lucene.store.MMapDirectory.map(MMapDirectory.java:283)
at org.apache.lucene.store.MMapDirectory$MMapIndexInput.<init>(MMapDirectory.java:228)
at org.apache.lucene.store.MMapDirectory.openInput(MMapDirectory.java:195)
at org.apache.lucene.index.SegmentInfos.read(SegmentInfos.java:285)
at org.apache.lucene.index.SegmentInfos$1.doBody(SegmentInfos.java:347)
at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:783)
at org.apache.lucene.index.SegmentInfos$FindSegmentsFile.run(SegmentInfos.java:630)
at org.apache.lucene.index.SegmentInfos.read(SegmentInfos.java:343)
at org.apache.lucene.index.StandardDirectoryReader.isCurrent(StandardDirectoryReader.java:326)
at org.apache.lucene.index.StandardDirectoryReader.doOpenNoWriter(StandardDirectoryReader.java:284)
at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:247)
at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:235)
at org.apache.lucene.index.DirectoryReader.openIfChanged(DirectoryReader.java:170)
at gatchan.jedit.lucene.AbstractIndex.initReader(AbstractIndex.java:141)
at gatchan.jedit.lucene.AbstractIndex.getSearcher(AbstractIndex.java:119)
at gatchan.jedit.lucene.CentralIndex.addFile(CentralIndex.java:128)
at gatchan.jedit.lucene.IndexImpl.addDocument(IndexImpl.java:385)
at gatchan.jedit.lucene.IndexImpl.addFiles(IndexImpl.java:151)
at gatchan.jedit.lucene.LucenePlugin.addToIndex(LucenePlugin.java:344)
at gatchan.jedit.lucene.IndexProjectAction$ProjectIndexer._run(IndexProjectAction.java:87)
at org.gjt.sp.util.Task.run(Task.java:64)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:744)
The solution to the problem is to catch the ClosedByInterruptException thrown by the Lucene library and terminate the thread gracefully.
I tried calling Thread.currentThread().interrupt(); at the point where the ClosedByInterruptException was thrown and that didn't work, so to close the thread gracefully I threw a new IndexInterruptedException to cleanly get back to the top level and exit the thread gracefully.
Most of the changes are just adding throws clauses to propogate the up the call stack, and by running the same check for the other calls to the Lucene library, so the changes are rather more extensive than I originally envisaged.
It looks like svn diff doesn't pick up new files so included is the new file.
Last edit: Tim Blackler 2014-12-29
sorry that should have said "caught by the existing catch block for IOException, which just logs the error and carries on processing, hence the task does not cancel."
amended post
Hi Guys, Have you had a chance to look at this ?
applied in trunk, thanks