Skip to content

Commit

Permalink
Merge pull request #209 from dmlloyd/fixes
Browse files Browse the repository at this point in the history 
Small fixes & cleanups for 3.0
  • Loading branch information
@jamezp
jamezp authored Nov 20, 2018
2 parents 5eb109b + 7513c82 commit 883b5f0
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 366 deletions.
92 changes: 23 additions & 69 deletions src/main/java/org/jboss/logmanager/ExtHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,14 @@
* An extended logger handler. Use this class as a base class for log handlers which require {@code ExtLogRecord}
* instances.
*/
public abstract class ExtHandler extends Handler implements FlushableCloseable, Protectable {
public abstract class ExtHandler extends Handler implements FlushableCloseable {

private static final Permission CONTROL_PERMISSION = new LoggingPermission("control", null);
private volatile boolean autoFlush = true;
private volatile boolean enabled = true;
private volatile boolean closeChildren;
private static final ErrorManager DEFAULT_ERROR_MANAGER = new OnlyOnceErrorManager();

@SuppressWarnings("unused")
private volatile Object protectKey;
private final ThreadLocal<Boolean> granted = new InheritableThreadLocal<Boolean>();

private static final AtomicReferenceFieldUpdater<ExtHandler, Object> protectKeyUpdater = AtomicReferenceFieldUpdater.newUpdater(ExtHandler.class, Object.class, "protectKey");

/**
* The sub-handlers for this handler. May only be updated using the {@link #handlersUpdater} atomic updater. The array
* instance should not be modified (treat as immutable).
Expand Down Expand Up @@ -112,10 +106,10 @@ protected void doPublish(final ExtLogRecord record) {
* @param handler the handler to add
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public void addHandler(Handler handler) throws SecurityException {
checkAccess(this);
checkAccess();
if (handler == null) {
throw new NullPointerException("handler is null");
}
Expand All @@ -128,10 +122,10 @@ public void addHandler(Handler handler) throws SecurityException {
* @param handler the handler to remove
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public void removeHandler(Handler handler) throws SecurityException {
checkAccess(this);
checkAccess();
if (handler == null) {
return;
}
Expand All @@ -154,10 +148,10 @@ public Handler[] getHandlers() {
* @return the old sub-handler array
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public Handler[] clearHandlers() throws SecurityException {
checkAccess(this);
checkAccess();
final Handler[] handlers = this.handlers;
handlersUpdater.clear(this);
return handlers.length > 0 ? handlers.clone() : handlers;
Expand All @@ -170,7 +164,7 @@ public Handler[] clearHandlers() throws SecurityException {
* @return the old sub-handler array
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public Handler[] setHandlers(final Handler[] newHandlers) throws SecurityException {
if (newHandlers == null) {
Expand All @@ -179,7 +173,7 @@ public Handler[] setHandlers(final Handler[] newHandlers) throws SecurityExcepti
if (newHandlers.length == 0) {
return clearHandlers();
} else {
checkAccess(this);
checkAccess();
final Handler[] handlers = handlersUpdater.getAndSet(this, newHandlers);
return handlers.length > 0 ? handlers.clone() : handlers;
}
Expand All @@ -200,10 +194,10 @@ public boolean isAutoFlush() {
* @param autoFlush {@code true} to automatically flush after each write; {@code false} otherwise
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public void setAutoFlush(final boolean autoFlush) throws SecurityException {
checkAccess(this);
checkAccess();
this.autoFlush = autoFlush;
if (autoFlush) {
flush();
Expand All @@ -216,10 +210,10 @@ public void setAutoFlush(final boolean autoFlush) throws SecurityException {
* @param enabled {@code true} to enable the handler or {@code false} to disable the handler.
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
public final void setEnabled(final boolean enabled) throws SecurityException {
checkAccess(this);
checkAccess();
this.enabled = enabled;
}

Expand Down Expand Up @@ -250,50 +244,15 @@ public boolean isCloseChildren() {
* is closed
*/
public void setCloseChildren(final boolean closeChildren) {
checkAccess(this);
checkAccess();
this.closeChildren = closeChildren;
}

@Override
public final void protect(Object protectionKey) throws SecurityException {
if (protectKeyUpdater.compareAndSet(this, null, protectionKey)) {
return;
}
throw new SecurityException("Log handler already protected");
}

@Override
public final void unprotect(Object protectionKey) throws SecurityException {
if (protectKeyUpdater.compareAndSet(this, protectionKey, null)) {
return;
}
throw accessDenied();
}

@Override
public final void enableAccess(Object protectKey) {
if (protectKey == this.protectKey) {
granted.set(Boolean.TRUE);
}
}

@Override
public final void disableAccess() {
granted.remove();
}

private static SecurityException accessDenied() {
return new SecurityException("Log handler modification access denied");
}

/**
* Check access.
*
* @deprecated use {@link #checkAccess(ExtHandler)}
*
* @throws SecurityException if a security manager is installed and the caller does not have the {@code "control" LoggingPermission}
*/
@Deprecated
protected static void checkAccess() throws SecurityException {
final SecurityManager sm = System.getSecurityManager();
if (sm != null) {
Expand All @@ -307,16 +266,11 @@ protected static void checkAccess() throws SecurityException {
* @param handler the handler to check access on.
*
* @throws SecurityException if a security manager exists and if the caller does not have {@code
* LoggingPermission(control)} or the handler is {@link #protect(Object) protected}.
* LoggingPermission(control)}
*/
@Deprecated
protected static void checkAccess(final ExtHandler handler) throws SecurityException {
final SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(CONTROL_PERMISSION);
}
if (handler.protectKey != null && handler.granted.get() == null) {
throw accessDenied();
}
checkAccess();
}

/**
Expand All @@ -336,7 +290,7 @@ public void flush() {
*/
@Override
public void close() throws SecurityException {
checkAccess(this);
checkAccess();
if (closeChildren) {
for (Handler handler : handlers)
try {
Expand All @@ -350,31 +304,31 @@ public void close() throws SecurityException {

@Override
public void setFormatter(final Formatter newFormatter) throws SecurityException {
checkAccess(this);
checkAccess();
super.setFormatter(newFormatter);
}

@Override
public void setFilter(final Filter newFilter) throws SecurityException {
checkAccess(this);
checkAccess();
super.setFilter(newFilter);
}

@Override
public void setEncoding(final String encoding) throws SecurityException, UnsupportedEncodingException {
checkAccess(this);
checkAccess();
super.setEncoding(encoding);
}

@Override
public void setErrorManager(final ErrorManager em) {
checkAccess(this);
checkAccess();
super.setErrorManager(em);
}

@Override
public void setLevel(final Level newLevel) throws SecurityException {
checkAccess(this);
checkAccess();
super.setLevel(newLevel);
}

Expand Down
56 changes: 6 additions & 50 deletions src/main/java/org/jboss/logmanager/LogContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import java.util.logging.Level;
import java.util.logging.LoggingMXBean;
import java.util.logging.LoggingPermission;
Expand All @@ -43,7 +42,7 @@
* A logging context, for producing isolated logging environments.
*/
@SuppressWarnings("unused")
public final class LogContext implements Protectable, AutoCloseable {
public final class LogContext implements AutoCloseable {
private static final LogContext SYSTEM_CONTEXT = new LogContext(false);

static final Permission CREATE_CONTEXT_PERMISSION = new RuntimePermission("createLogContext", null);
Expand All @@ -56,13 +55,6 @@ public final class LogContext implements Protectable, AutoCloseable {
private final boolean strong;
private final ConcurrentSkipListMap<String, AtomicInteger> loggerNames;

@SuppressWarnings("unused")
private volatile Object protectKey;

private final ThreadLocal<Boolean> granted = new InheritableThreadLocal<Boolean>();

private static final AtomicReferenceFieldUpdater<LogContext, Object> protectKeyUpdater = AtomicReferenceFieldUpdater.newUpdater(LogContext.class, Object.class, "protectKey");

/**
* This lazy holder class is required to prevent a problem due to a LogContext instance being constructed
* before the class init is complete.
Expand Down Expand Up @@ -318,34 +310,6 @@ public static void setLogContextSelector(LogContextSelector newSelector) {
logContextSelector = newSelector;
}

@Override
public void protect(Object protectionKey) throws SecurityException {
if (protectKeyUpdater.compareAndSet(this, null, protectionKey)) {
return;
}
throw new SecurityException("Log context already protected");
}

@Override
public void unprotect(Object protectionKey) throws SecurityException {
if (protectKeyUpdater.compareAndSet(this, protectionKey, null)) {
return;
}
throw accessDenied();
}

@Override
public void enableAccess(Object protectKey) throws SecurityException {
if (protectKey == this.protectKey) {
granted.set(Boolean.TRUE);
}
}

@Override
public void disableAccess() {
granted.remove();
}

@Override
public void close() throws Exception {
synchronized (treeLock) {
Expand Down Expand Up @@ -467,20 +431,13 @@ private static SecurityException accessDenied() {
return new SecurityException("Log context modification access denied");
}

static void checkSecurityAccess() {
static void checkAccess() {
final SecurityManager sm = System.getSecurityManager();
if (sm != null) {
sm.checkPermission(CONTROL_PERMISSION);
}
}

static void checkAccess(final LogContext logContext) {
checkSecurityAccess();
if (logContext.protectKey != null && logContext.granted.get() == null) {
throw accessDenied();
}
}

LoggerNode getRootLoggerNode() {
return rootLogger;
}
Expand All @@ -490,12 +447,11 @@ ConcurrentMap<String, LoggerNode> createChildMap() {
}

private void recursivelyClose(final LoggerNode loggerNode) {
synchronized (treeLock) {
for (LoggerNode child : loggerNode.getChildren()) {
recursivelyClose(child);
}
loggerNode.close();
assert Thread.holdsLock(treeLock);
for (LoggerNode child : loggerNode.getChildren()) {
recursivelyClose(child);
}
loggerNode.close();
}

private interface LevelRef {
Expand Down
Loading

0 comments on commit 883b5f0

Please sign in to comment.