System.out is used as logger in org.apache.parquet.Log

[asfimport]

The use of System.out in the StreamHandler at line 62 in the org.apache.parquet.Log provoke that the java.util.logging.LogManager will close System.out in case the reset method is called on the LogManager.
This is special problematic when Shutdown Hooks are used in a project, as the LogManager set-up one, there call the reset method, and for this reason a race condition exist on System.out, if used in the project Shutdown Hook.
Other scenarios might also exist where a program call the LogManager reset method.

An eventual solution is to NOT use java.util.logging.Logger at all in the Parquet environment but use either log4j or slf4j as used in almost every other part in the Hadoop environment, this would also allow the user to control the logging much better than today.

Environment: parquet-common-1.8.1.jar
Reporter: Henrik Baastrup
Assignee: Ryan Blue / @rdblue

Related issues:

• Logger instantiated for package org.apache.parquet may be GC-ed (requires)

_{Note: This issue was originally created as PARQUET-395. Please see the migration documentation for further details.}

[asfimport]

Henrik Baastrup:

Workaround

The below code is noway beautiful but will work as workaround and should be implemented before you call any API in the Parquet library.

import java.util.loggingLogger;
import org.apache.commons.io.output.NullOutputStream;
import org.apache.parquet.Log;
import java.util.logging.SimpleFormatter;
import java.util.logging.StreamHandler;

OffStreamHandler offH = new OffStreamHandler(new NullOutputStream(), new SimpleFormatter());
Logger logger = Logger.getLogger(Log.class.getPackage().getName());
logger.addHandler(offH);

public class OffStreamHandler extends StreamHandler {
	private Level myLevel = Level.OFF;  // by default, put out off

	/**
	  * The only method we really change to check whether the message
	  * is smaller than maxlevel.
	  */
	@Override
	public synchronized void publish(LogRecord record) {
	    if (record.getLevel().intValue() > this.myLevel.intValue()) {
	        // do nothing if the level is above myLevel
	    } else {
	            // if we arrived here, do what we always do
	            super.publish(record);
	            super.flush();
	    }
	}
	    
	@Override
	public synchronized void close() throws SecurityException {
		super.close();
	}

	public void setLevel(Level l) {
		myLevel = l;
	}

	/** Constructor forwarding */
	public OffStreamHandler(OutputStream out, Formatter formatter) {
	    super(out, formatter);
	}

	/** Constructor forwarding */
	public OffStreamHandler() {
	    super();
	}
}

[asfimport]

Ryan Blue / @rdblue:
Thanks for reporting this, [~hbaastrup]. I think you're right that we should be using slf4j and I have an open pull request for PARQUET-305 that implements that. I'd like to get that in for the 1.9.0 release. I'll leave this open until we get that done.

[asfimport]

Henrik Baastrup:
Sounds great with the integration of slf4j.
But in the mean time I think it would be a good idea to drop the log handler there use the System.out in the org.apache.parquet.Log, because as is today, a user of the library risk to close this stream, without to know it.
If you need any help I'm ready to give a hand.

[asfimport]

Ryan Blue / @rdblue:
Moving to SLF4J was implemented in PARQUET-305. This should be fixed by configuring the SLF4J bindings used by applications.