Skip to content

Commit

Permalink
Merge pull request #24 from jhspaybar/fixdeadlock
Browse files Browse the repository at this point in the history 
Address deadlock when attempting to avoid stack creation.
  • Loading branch information
@jhspaybar
jhspaybar committed Oct 17, 2017
2 parents e64fe49 + fa0110e commit f571bcc
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 137 deletions.
104 changes: 21 additions & 83 deletions src/main/java/com/netflix/blitz4j/LoggingContext.java
View file
Expand Up @@ -16,24 +16,16 @@

package com.netflix.blitz4j;

import com.netflix.logging.log4jAdapter.NFPatternLayout;
import com.netflix.servo.monitor.Monitors;
import com.netflix.servo.monitor.Stopwatch;
import com.netflix.servo.monitor.Timer;
import org.apache.log4j.Appender;
import org.apache.log4j.Category;
import org.apache.log4j.Level;
import org.apache.log4j.MDC;
import org.apache.log4j.spi.AppenderAttachable;
import org.apache.log4j.spi.LocationInfo;
import org.apache.log4j.spi.LoggingEvent;

import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

Expand Down Expand Up @@ -170,7 +162,7 @@ public LocationInfo generateLocationInfo(LoggingEvent event) {
try {
// We should only generate location info if the caller is using NFPatternLayout otherwise this is expensive and unused.
if (isUsingNFPatternLayout(event.getLogger())) {
locationInfo = (LocationInfo) LoggingContext
locationInfo = LoggingContext
.getInstance()
.getLocationInfo(Class.forName(event.getFQNOfLoggerClass()));
if (locationInfo != null) {
Expand All @@ -186,85 +178,31 @@ public LocationInfo generateLocationInfo(LoggingEvent event) {
return locationInfo;
}

private boolean isUsingNFPatternLayout(Category logger) {
if (logger == null) {
return false;
}

public void shouldGenerateLocationInfo(Category logger) {
HashSet<Category> loggerNeedsLocation = loggerNeedsLocationRef.get();
// If we've already seen this logger and it needs location info, assume it still does.
// Due to reconfiguration, it's possible it doesn't anymore, but this is rare so we optimize
// for a fast return on loggers previously known to need location info.
if (loggerNeedsLocation.contains(logger)) {
return true;
}

// If any of the appenders in the tree below need location information remember this logger and return.
if (isUsingNFPatternLayout(logger.getAllAppenders())) {
do {
HashSet<Category> copy = new HashSet<>(loggerNeedsLocation);
copy.add(logger);
if (loggerNeedsLocationRef.compareAndSet(loggerNeedsLocation, copy)) {
return true;
}
loggerNeedsLocation = loggerNeedsLocationRef.get();
} while(true);
}

// If this is not an additive logger, our search is done, otherwise we must look at parents.
if (!logger.getAdditivity()) {
return false;
}

Category parentLogger = logger.getParent();
if (parentLogger == null) {
return false;
}

// Now we need to traverse all parents and remember the top level logger whose parents need
// location info if additivity was set to true.
if(isUsingNFPatternLayout(parentLogger)) {
do {
HashSet<Category> copy = new HashSet<>(loggerNeedsLocation);
copy.add(logger);
if (loggerNeedsLocationRef.compareAndSet(loggerNeedsLocation, copy)) {
return true;
}
loggerNeedsLocation = loggerNeedsLocationRef.get();
} while(true);
}

// An exhaustive search returned nothing. We want location information to show up when
// a reconfiguration occurs so we don't cache the result. This is still a much cheaper
// cost than generating a stack trace and so we are happy to pay it every time we log
// if we don't actually need the location information.
return false;

}

private boolean isUsingNFPatternLayout(Enumeration enumeration) {
if (enumeration == null) {
return false;
}

while(enumeration.hasMoreElements()) {
Object maybeAppender = enumeration.nextElement();
if (maybeAppender instanceof Appender) {
Appender a = (Appender) maybeAppender;
if (a.getLayout() instanceof NFPatternLayout) {
return true;
}
// Add the logger to the set of loggers that needs location info.
do {
// If we've already seen this logger return immediately.
if (loggerNeedsLocation.contains(logger)) {
return;
}

if (maybeAppender instanceof AppenderAttachable) {
AppenderAttachable aa = (AppenderAttachable) maybeAppender;
if (isUsingNFPatternLayout(aa.getAllAppenders())) {
return true;
}
// Try to add the logger
HashSet<Category> copy = new HashSet<>(loggerNeedsLocation);
copy.add(logger);
if (loggerNeedsLocationRef.compareAndSet(loggerNeedsLocation, copy)) {
return;
}
}

return false;
// If there's a conflict, pull the map out and try again.
loggerNeedsLocation = loggerNeedsLocationRef.get();
} while(true);
}

private boolean isUsingNFPatternLayout(Category logger) {
// Assume we don't need location info until proven otherwise
return logger != null && loggerNeedsLocationRef.get().contains(logger);
}

/**
Expand Down Expand Up @@ -311,7 +249,7 @@ public void clearContextLevel() {

/**
* Get the context {@link Level} for the request-based logging
* @param level - The level of logging to be enabled for this request
* @return level - The level of logging to be enabled for this request
*/
public Level getContextLevel() {
return (Level)MDC.get(CONTEXT_LEVEL);
Expand Down
108 changes: 54 additions & 54 deletions src/main/java/com/netflix/logging/log4jAdapter/NFPatternParser.java
View file
Expand Up @@ -33,66 +33,66 @@
* as class, line number etc.
*
* @author Karthik Ranganathan
*
*/
public class NFPatternParser extends PatternParser {
private static List<Character> contextCharList = Arrays.asList(Character.valueOf('c'),
Character.valueOf('l'),
Character.valueOf('M'),
Character.valueOf('C'),
Character.valueOf('L'),
Character.valueOf('F'));
private static List<Character> contextCharList = Arrays.asList(Character.valueOf('c'),
Character.valueOf('l'),
Character.valueOf('M'),
Character.valueOf('C'),
Character.valueOf('L'),
Character.valueOf('F'));

public NFPatternParser(String pattern) {
super(pattern);

}

protected void finalizeConverter(char c) {
if (contextCharList.contains(Character.valueOf(c))) {
PatternConverter pc = new NFPatternConverter(formattingInfo, c);
addConverter(pc);
currentLiteral.setLength(0);
} else {
super.finalizeConverter(c);
}
}

public NFPatternParser(String pattern) {
super(pattern);

}
private static class NFPatternConverter extends PatternConverter {
private char type;

protected void finalizeConverter(char c) {
if (contextCharList.contains(Character.valueOf(c))) {
PatternConverter pc = new NFPatternConverter(formattingInfo, c);
addConverter(pc);
currentLiteral.setLength(0);
} else {
super.finalizeConverter(c);
}
}
NFPatternConverter(FormattingInfo formattingInfo, char type) {
super(formattingInfo);
this.type = type;
}

private static class NFPatternConverter extends PatternConverter {
private char type;
@Override
public String convert(LoggingEvent event) {
LoggingContext.getInstance().shouldGenerateLocationInfo(event.getLogger());
LocationInfo locationInfo = LoggingContext.getInstance().getLocationInfo(event);
if (locationInfo == null) {
return "";
}
switch (type) {
case 'M':
return locationInfo.getMethodName();
case 'c':
return event.getLoggerName();
case 'C':
return locationInfo.getClassName();
case 'L':
return locationInfo.getLineNumber();
case 'l':
return (locationInfo.getFileName() + ":"
+ locationInfo.getClassName() + " "
+ locationInfo.getLineNumber() + " " + locationInfo
.getMethodName());
case 'F':
return locationInfo.getFileName();
}
return "";

NFPatternConverter(FormattingInfo formattingInfo, char type) {
super(formattingInfo);
this.type = type;
}
}

@Override
public String convert(LoggingEvent event) {
LocationInfo locationInfo = LoggingContext.getInstance().getLocationInfo(event);
if (locationInfo == null) {
return "";
}
switch (type) {
case 'M':
return locationInfo.getMethodName();
case 'c':
return event.getLoggerName();
case 'C':
return locationInfo.getClassName();
case 'L':
return locationInfo.getLineNumber();
case 'l':
return (locationInfo.getFileName() + ":"
+ locationInfo.getClassName() + " "
+ locationInfo.getLineNumber() + " " + locationInfo
.getMethodName());
case 'F':
return locationInfo.getFileName();
}
return "";

}


}
}
}

0 comments on commit f571bcc

Please sign in to comment.