[GROOVY-8546] Parrot Parser: multiple Reader instances opened from SourceUnit; many left open

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.6.0-alpha-3, 3.0.0-alpha-2
• Fix Version/s: 2.6.0-alpha-4, 3.0.0-alpha-3
• Component/s: parser
• Assignee: Daniel Sun
• Reporter: Eric Milles
• Created: 16/Apr/18 00:21
• Updated: 26/Jul/18 04:46
• Resolved: 17/Apr/18 06:01

Description

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.

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();
    }
}