OutOfMemoryError caused by UnixProcess holding all output written to stdout

[cerebrotecnologico]

The class org.openqa.selenium.os.UnixProcess contains the following method:

private final ByteArrayOutputStream inputOut = new ByteArrayOutputStream();

private OutputStream getOutputStream() {
return drainTo == null ? inputOut
: new MultioutputStream(inputOut, drainTo);
}

The variable inputOut is a ByteArrayOutputStream, holding anything that is written in stdout into an array of bytes. The contents are never eliminated, therefore, all output is accumulated, eventually producing the OutOfMemoryError.

[cerebrotecnologico]

Hello,
The solution won't solve the OOM error. A new CommandLine is already created every time that a FirefoxBinary is created, CommandLine creates a UnixProcess which creates a new ByteArrayOutputStream.

I see no reason to keep the output into a ByteArrayOutputStream, specially if the output is directed into another stream (drainTo) which is set by the environment variable "webdriver.firefox.log".

Seems to me that the only purpose of the inputOut variable is perhaps for some debugging of UnixProcess, since it is only used by getStdOut(), which returns the contents of all the console output ever produced.

Could we eliminate the method getStdOut() from the class UnixProcess ?

The variable inputOut could be set to System.out (so, by default printing the output of the plugins calling console.print) or something equivalent to org.apache.commons.io.output.NullOutputStream (in this case, by default the output would go into a black hole )

Alternatively, inputOut could keep just the last N bytes of output rather holding the entire console output ever produced.

[lukeis]

@cerebrotecnologico that pull request was abandoned and no one has submitted another one. Pull requests appreciated? :)

[ROARy]

@cerebrotecnologico another idea would be to let the user of UnixProcess decide what kind of outputstream they want with a setter and set the default to NullOutputStream. An OOM error could still happen if someone uses a ByteArrayOutputStream but then at least the user has control over that.

Generally, isn't it bad to eliminate a public method like getStdOut() from the class's interface?

Side note, "drainTo" isn't actually combined with "inputOut" at any point. They're held and written to in parallel inside a MultioutputStream, for some reason.

[barancev]

I think we can change ByteArrayOutputStream to CircularOutputStream.

[barancev]

Fixed by commit d4a21ae

[jsdevel]

I believe the documentation on running firefox needs to be added with d4a21ae. I'm not getting this error locally:

java.lang.NoClassDefFoundError: org/openqa/selenium/io/CircularOutputStream

[jsdevel]

Actually, which maven dependency has org.openqa.selenium.io.CircularOutputStream? I'm not seeing it in standalone.

[lukeis]

@jsdevel it's in selenium-remote-driver - http://repo1.maven.org/maven2/org/seleniumhq/selenium/selenium-remote-driver/2.52.0/selenium-remote-driver-2.52.0.jar

(and it's included in the standalone jar)

[jsdevel]

I don't see it in standalone.

[lukeis]

@jsdevel i just downloaded again - http://selenium-release.storage.googleapis.com/2.52/selenium-server-standalone-2.52.0.jar

and "org/openqa/selenium/io/CircularOutputStream.class" is in it.