[jira] [Commented] (GROOVY-8546) Parrot Parser: multiple Reader instances opened from SourceUnit; many left open
[
https://issues.apache.org/jira/browse/GROOVY-8546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16440281#comment-16440281
]
Daniel Sun commented on GROOVY-8546:
{quote}It would help if you linked to the relevant source code since the
groovy-all sources jar does not carry the Antlr source.
{quote}
Here is the source code of antlr4's {{fromReader}}:
{code:java}
/**
* Creates a {@link CharStream} given a {@link Reader} and its
* source name. Closes the reader before returning.
*/
public static CodePointCharStream fromReader(Reader r, String
sourceName) throws IOException {
try {
CodePointBuffer.Builder codePointBufferBuilder =
CodePointBuffer.builder(DEFAULT_BUFFER_SIZE);
CharBuffer charBuffer =
CharBuffer.allocate(DEFAULT_BUFFER_SIZE);
while ((r.read(charBuffer)) != -1) {
charBuffer.flip();
codePointBufferBuilder.append(charBuffer);
charBuffer.compact();
}
return
CodePointCharStream.fromBuffer(codePointBufferBuilder.build(), sourceName);
}
finally {
r.close();
}
}
{code}
I verified that *only the reader created by `getReader` method in
{{Antlr4ParserPlugin}} is not closed*, here is the commit to fix it:
https://github.com/apache/groovy/commit/e19a93242dcc204cd94e034242a348c113cc1130
As for other Reader/Stream instances, they have been closed, you have to
investigate the relevant code to check.
> Parrot Parser: multiple Reader instances opened from SourceUnit; many left
> open
> ---
>
> Key: GROOVY-8546
> URL: https://issues.apache.org/jira/browse/GROOVY-8546
> Project: Groovy
> Issue Type: Bug
> Components: parser
>Affects Versions: 2.6.0-alpha-3, 3.0.0-alpha-2
>Reporter: Eric Milles
>Priority: Major
>
> {{Antlr4ParserPlugin}} makes very inefficient use of
> {{ReaderSource}}/{{SourceUnit}}. {{parseCST}} is passed a {{Reader}} with
> the idea that it is the source of character data -- and this reader is closed
> from outside. It ignores this and mines the passed {{SourceUnit}}, which
> seems unnecessary since the same reference is passed again in {{buildAST}}.
> Both {{parseCST}} and {{buildAST}} call {{getReader}}, which opens a buffered
> reader on the source unit file and never closes them. Lastly, {{AstBuilder}}
> calls {{getReader}} as well to create a {{CharStream}} and never closes this
> reader.
> Doing this in a long-running process (like an IDE) is causing numerous
> problems due to open file handles.
> {code:java}
> public class Antlr4ParserPlugin implements ParserPlugin {
> private ReaderSource readerSource;
> @Override
> public Reduction parseCST(SourceUnit sourceUnit, java.io.Reader reader)
> throws CompilationFailedException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null != readerSource && null != readerSource.getReader()) {
> this.readerSource = readerSource;
> } else {
> this.readerSource = new
> StringReaderSource(IOGroovyMethods.getText(reader),
> sourceUnit.getConfiguration());
> }
> } catch (IOException e) {
> throw new GroovyBugError("Failed to create StringReaderSource
> instance", e);
> }
> return null;
> }
> @Override
> public ModuleNode buildAST(SourceUnit sourceUnit, ClassLoader
> classLoader, Reduction cst) throws ParserException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null == readerSource || null == readerSource.getReader()) {
> sourceUnit.setSource(this.readerSource);
> }
> } catch (IOException e) {
> sourceUnit.setSource(this.readerSource);
> }
> AstBuilder builder = new AstBuilder(sourceUnit);
> return builder.buildAST();
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8546) Parrot Parser: multiple Reader instances opened from SourceUnit; many left open
[
https://issues.apache.org/jira/browse/GROOVY-8546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16439816#comment-16439816
]
Eric Milles commented on GROOVY-8546:
-
It would help if you linked to the relevant source code since the groovy-all
sources jar does not carry the Antlr source. Even if the reader used in
AstBuilder is closed properly, there are at least 2 opened readers in
Antlr4ParserPlugin that are not closed. And there is a reader opened outside
Antlr4ParserPlugin and not used, which is inefficient.
> Parrot Parser: multiple Reader instances opened from SourceUnit; many left
> open
> ---
>
> Key: GROOVY-8546
> URL: https://issues.apache.org/jira/browse/GROOVY-8546
> Project: Groovy
> Issue Type: Bug
> Components: parser
>Affects Versions: 2.6.0-alpha-3, 3.0.0-alpha-2
>Reporter: Eric Milles
>Priority: Major
>
> {{Antlr4ParserPlugin}} makes very inefficient use of
> {{ReaderSource}}/{{SourceUnit}}. {{parseCST}} is passed a {{Reader}} with
> the idea that it is the source of character data -- and this reader is closed
> from outside. It ignores this and mines the passed {{SourceUnit}}, which
> seems unnecessary since the same reference is passed again in {{buildAST}}.
> Both {{parseCST}} and {{buildAST}} call {{getReader}}, which opens a buffered
> reader on the source unit file and never closes them. Lastly, {{AstBuilder}}
> calls {{getReader}} as well to create a {{CharStream}} and never closes this
> reader.
> Doing this in a long-running process (like an IDE) is causing numerous
> problems due to open file handles.
> {code:java}
> public class Antlr4ParserPlugin implements ParserPlugin {
> private ReaderSource readerSource;
> @Override
> public Reduction parseCST(SourceUnit sourceUnit, java.io.Reader reader)
> throws CompilationFailedException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null != readerSource && null != readerSource.getReader()) {
> this.readerSource = readerSource;
> } else {
> this.readerSource = new
> StringReaderSource(IOGroovyMethods.getText(reader),
> sourceUnit.getConfiguration());
> }
> } catch (IOException e) {
> throw new GroovyBugError("Failed to create StringReaderSource
> instance", e);
> }
> return null;
> }
> @Override
> public ModuleNode buildAST(SourceUnit sourceUnit, ClassLoader
> classLoader, Reduction cst) throws ParserException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null == readerSource || null == readerSource.getReader()) {
> sourceUnit.setSource(this.readerSource);
> }
> } catch (IOException e) {
> sourceUnit.setSource(this.readerSource);
> }
> AstBuilder builder = new AstBuilder(sourceUnit);
> return builder.buildAST();
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8546) Parrot Parser: multiple Reader instances opened from SourceUnit; many left open
[
https://issues.apache.org/jira/browse/GROOVY-8546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16438916#comment-16438916
]
Daniel Sun commented on GROOVY-8546:
{{CharStreams.fromReader}} in the {{AstBuilder.createCharStream}} method will
close the reader held by {{sourceUnit, please see the source code of antlr4}}
{code:java}
private CharStream createCharStream(SourceUnit sourceUnit) {
CharStream charStream;
try {
charStream = CharStreams.fromReader(
new BufferedReader(sourceUnit.getSource().getReader()),
sourceUnit.getName());
} catch (IOException e) {
throw new RuntimeException("Error occurred when reading source
code.", e);
}
return charStream;
}
{code}
> Parrot Parser: multiple Reader instances opened from SourceUnit; many left
> open
> ---
>
> Key: GROOVY-8546
> URL: https://issues.apache.org/jira/browse/GROOVY-8546
> Project: Groovy
> Issue Type: Bug
> Components: parser
>Affects Versions: 2.6.0-alpha-3, 3.0.0-alpha-2
>Reporter: Eric Milles
>Priority: Major
>
> {{Antlr4ParserPlugin}} makes very inefficient use of
> {{ReaderSource}}/{{SourceUnit}}. {{parseCST}} is passed a {{Reader}} with
> the idea that it is the source of character data -- and this reader is closed
> from outside. It ignores this and mines the passed {{SourceUnit}}, which
> seems unnecessary since the same reference is passed again in {{buildAST}}.
> Both {{parseCST}} and {{buildAST}} call {{getReader}}, which opens a buffered
> reader on the source unit file and never closes them. Lastly, {{AstBuilder}}
> calls {{getReader}} as well to create a {{CharStream}} and never closes this
> reader.
> Doing this in a long-running process (like an IDE) is causing numerous
> problems due to open file handles.
> {code:java}
> public class Antlr4ParserPlugin implements ParserPlugin {
> private ReaderSource readerSource;
> @Override
> public Reduction parseCST(SourceUnit sourceUnit, java.io.Reader reader)
> throws CompilationFailedException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null != readerSource && null != readerSource.getReader()) {
> this.readerSource = readerSource;
> } else {
> this.readerSource = new
> StringReaderSource(IOGroovyMethods.getText(reader),
> sourceUnit.getConfiguration());
> }
> } catch (IOException e) {
> throw new GroovyBugError("Failed to create StringReaderSource
> instance", e);
> }
> return null;
> }
> @Override
> public ModuleNode buildAST(SourceUnit sourceUnit, ClassLoader
> classLoader, Reduction cst) throws ParserException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null == readerSource || null == readerSource.getReader()) {
> sourceUnit.setSource(this.readerSource);
> }
> } catch (IOException e) {
> sourceUnit.setSource(this.readerSource);
> }
> AstBuilder builder = new AstBuilder(sourceUnit);
> return builder.buildAST();
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GROOVY-8546) Parrot Parser: multiple Reader instances opened from SourceUnit; many left open
[
https://issues.apache.org/jira/browse/GROOVY-8546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16438915#comment-16438915
]
Daniel Sun commented on GROOVY-8546:
{{CharStream}} of antlr4 is not a IO stream, so we need not close it.
> Parrot Parser: multiple Reader instances opened from SourceUnit; many left
> open
> ---
>
> Key: GROOVY-8546
> URL: https://issues.apache.org/jira/browse/GROOVY-8546
> Project: Groovy
> Issue Type: Bug
> Components: parser
>Affects Versions: 2.6.0-alpha-3, 3.0.0-alpha-2
>Reporter: Eric Milles
>Priority: Major
>
> {{Antlr4ParserPlugin}} makes very inefficient use of
> {{ReaderSource}}/{{SourceUnit}}. {{parseCST}} is passed a {{Reader}} with
> the idea that it is the source of character data -- and this reader is closed
> from outside. It ignores this and mines the passed {{SourceUnit}}, which
> seems unnecessary since the same reference is passed again in {{buildAST}}.
> Both {{parseCST}} and {{buildAST}} call {{getReader}}, which opens a buffered
> reader on the source unit file and never closes them. Lastly, {{AstBuilder}}
> calls {{getReader}} as well to create a {{CharStream}} and never closes this
> reader.
> Doing this in a long-running process (like an IDE) is causing numerous
> problems due to open file handles.
> {code:java}
> public class Antlr4ParserPlugin implements ParserPlugin {
> private ReaderSource readerSource;
> @Override
> public Reduction parseCST(SourceUnit sourceUnit, java.io.Reader reader)
> throws CompilationFailedException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null != readerSource && null != readerSource.getReader()) {
> this.readerSource = readerSource;
> } else {
> this.readerSource = new
> StringReaderSource(IOGroovyMethods.getText(reader),
> sourceUnit.getConfiguration());
> }
> } catch (IOException e) {
> throw new GroovyBugError("Failed to create StringReaderSource
> instance", e);
> }
> return null;
> }
> @Override
> public ModuleNode buildAST(SourceUnit sourceUnit, ClassLoader
> classLoader, Reduction cst) throws ParserException {
> try {
> ReaderSource readerSource = sourceUnit.getSource();
> if (null == readerSource || null == readerSource.getReader()) {
> sourceUnit.setSource(this.readerSource);
> }
> } catch (IOException e) {
> sourceUnit.setSource(this.readerSource);
> }
> AstBuilder builder = new AstBuilder(sourceUnit);
> return builder.buildAST();
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
