Skip to content

Commit ab31e20

Browse files
authored
Fix logger factory lazy initialization problem (rabbitmq#133)
1 parent 115f325 commit ab31e20

File tree

8 files changed

+139
-14
lines changed

8 files changed

+139
-14
lines changed

pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
<groupId>com.alipay.sofa.common</groupId>
66
<artifactId>sofa-common-tools</artifactId>
7-
<version>1.3.2</version>
7+
<version>1.3.3</version>
88
<packaging>jar</packaging>
99

1010
<name>${project.groupId}:${project.artifactId}</name>

src/main/java/com/alipay/sofa/common/log/LoggerSpaceManager.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.slf4j.ILoggerFactory;
2323
import org.slf4j.Logger;
2424

25+
import java.util.Collection;
26+
import java.util.Collections;
2527
import java.util.Map;
2628

2729
/**
@@ -41,7 +43,7 @@ public class LoggerSpaceManager {
4143
* @return logger of org.slf4j.Logger type
4244
*/
4345
public static Logger getLoggerBySpace(String name, String spaceName) {
44-
return MultiAppLoggerSpaceManager.getLoggerBySpace(name, spaceName);
46+
return getLoggerBySpace(name, new SpaceId(spaceName), Collections.emptyMap());
4547
}
4648

4749
/**

src/main/java/com/alipay/sofa/common/log/MultiAppLoggerSpaceManager.java

+23-9
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.alipay.sofa.common.log.factory.LoggerSpaceFactory4Log4jBuilder;
2525
import com.alipay.sofa.common.log.factory.LoggerSpaceFactory4LogbackBuilder;
2626
import com.alipay.sofa.common.log.factory.LoggerSpaceFactoryBuilder;
27+
import com.alipay.sofa.common.log.proxy.TemporaryILoggerFactoryPool;
2728
import com.alipay.sofa.common.space.SpaceId;
2829
import com.alipay.sofa.common.utils.ClassLoaderUtil;
2930
import com.alipay.sofa.common.utils.ReportUtil;
@@ -111,12 +112,7 @@ static void doInit(String spaceName, Map<String, String> props, ClassLoader spac
111112
*/
112113
static void doInit(SpaceId spaceId, Map<String, String> props, ClassLoader spaceClassloader) {
113114
LogSpace logSpace = new LogSpace(props, spaceClassloader);
114-
115-
AbstractLoggerSpaceFactory loggerSpaceFactory = createILoggerFactory(spaceId, logSpace,
116-
spaceClassloader);
117-
logSpace.setAbstractLoggerSpaceFactory(loggerSpaceFactory);
118-
119-
LOG_FACTORY_MAP.put(spaceId, logSpace);
115+
LOG_FACTORY_MAP.putIfAbsent(spaceId, logSpace);
120116
}
121117

122118
/**
@@ -187,11 +183,24 @@ public static Logger getLoggerBySpace(String name, com.alipay.sofa.common.log.Sp
187183
private static AbstractLoggerSpaceFactory getILoggerFactoryBySpaceName(SpaceId spaceId,
188184
ClassLoader spaceClassloader) {
189185
if (!isSpaceInitialized(spaceId)) {
190-
// If get logger without initializing, log space will be initialized with empty config map
191-
init(spaceId, null, spaceClassloader);
186+
return TemporaryILoggerFactoryPool.get(spaceId, spaceClassloader);
192187
}
193188

194-
return LOG_FACTORY_MAP.get(spaceId).getAbstractLoggerSpaceFactory();
189+
AbstractLoggerSpaceFactory factory = NOP_LOGGER_FACTORY;
190+
LogSpace space = LOG_FACTORY_MAP.get(spaceId);
191+
if (!isSpaceILoggerFactoryExisted(spaceId)) {
192+
synchronized (space) {
193+
if (!isSpaceILoggerFactoryExisted(spaceId)) {
194+
factory = createILoggerFactory(spaceId, space, spaceClassloader);
195+
space.setAbstractLoggerSpaceFactory(factory);
196+
}
197+
}
198+
} else {
199+
factory = LOG_FACTORY_MAP.get(spaceId).getAbstractLoggerSpaceFactory();
200+
}
201+
202+
203+
return factory;
195204
}
196205

197206
public static Logger setLoggerLevel(String loggerName, String spaceName,
@@ -258,6 +267,11 @@ public static boolean isSpaceInitialized(com.alipay.sofa.common.log.SpaceId spac
258267
return LOG_FACTORY_MAP.containsKey(spaceId);
259268
}
260269

270+
private static boolean isSpaceILoggerFactoryExisted(SpaceId spaceId) {
271+
return isSpaceInitialized(spaceId)
272+
&& LOG_FACTORY_MAP.get(spaceId).getAbstractLoggerSpaceFactory() != null;
273+
}
274+
261275
@Deprecated
262276
public static Map getSpacesMap() {
263277
return Collections.emptyMap();

src/main/java/com/alipay/sofa/common/log/proxy/LoggerProxy.java

-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
/**
2424
* Created by [email protected] on 2016/12/1.
2525
*/
26-
@Deprecated
2726
public class LoggerProxy implements Logger {
2827

2928
private TemporaryILoggerFactory.LoggerSelector loggerSelector;

src/main/java/com/alipay/sofa/common/log/proxy/TemporaryILoggerFactory.java

-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
* space
2727
* Created by [email protected] on 2016/12/1.
2828
*/
29-
@Deprecated
3029
public class TemporaryILoggerFactory extends AbstractLoggerSpaceFactory {
3130

3231
private final String space;

src/main/java/com/alipay/sofa/common/log/proxy/TemporaryILoggerFactoryPool.java

-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
/**
2727
* Created by [email protected] on 2016/12/5.
2828
*/
29-
@Deprecated
3029
public class TemporaryILoggerFactoryPool {
3130

3231
private static final ConcurrentHashMap<SpaceIdWithClassloader, TemporaryILoggerFactory> temporaryILoggerFactoryMap = new ConcurrentHashMap<SpaceIdWithClassloader, TemporaryILoggerFactory>();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package com.alipay.sofa.common.log;
18+
19+
import org.junit.Assert;
20+
import org.junit.Test;
21+
import org.slf4j.Logger;
22+
23+
import java.io.File;
24+
import java.nio.charset.StandardCharsets;
25+
import java.nio.file.Files;
26+
import java.nio.file.Paths;
27+
import java.util.HashMap;
28+
import java.util.List;
29+
import java.util.Map;
30+
31+
/**
32+
* @author <a href="mailto:[email protected]">Alaneuler</a>
33+
* Created on 2021/3/3
34+
*/
35+
public class LazyLogFactoryInitializingTest {
36+
public static final String SPACE_NAME = "lazy.init";
37+
38+
static {
39+
System.setProperty(Constants.LOGBACK_MIDDLEWARE_LOG_DISABLE_PROP_KEY, "true");
40+
}
41+
42+
public static Logger logger = MultiAppLoggerSpaceManager.getLoggerBySpace(
43+
"LAZY-INIT", SPACE_NAME);
44+
45+
@Test
46+
public void test() throws Exception {
47+
logger.info("test1");
48+
logger.info("test2");
49+
logger.info("test3");
50+
logger.info("test4");
51+
52+
File directory = new File(Constants.LOGGING_PATH_DEFAULT + "/lazy");
53+
if (directory.isDirectory()) {
54+
File[] files = directory.listFiles();
55+
if (files != null) {
56+
for (File f : files) {
57+
if (f.getName().startsWith("monitor.log")) {
58+
Assert.fail();
59+
}
60+
}
61+
}
62+
}
63+
64+
Map<String, String> props = new HashMap<>();
65+
props.put("logging.path." + SPACE_NAME, "./logs/");
66+
props.put("logging.level." + SPACE_NAME, "INFO");
67+
props.put("date.pattern." + SPACE_NAME, "yyyy-MM-dd");
68+
MultiAppLoggerSpaceManager.init(SPACE_NAME, props);
69+
70+
try {
71+
Files.write(Paths.get("./logs/lazy/monitor.log"), "".getBytes(StandardCharsets.UTF_8));
72+
} catch (Throwable e) {
73+
// just ignore
74+
}
75+
logger.info("test1");
76+
logger.info("test2");
77+
logger.info("test3");
78+
logger.info("test4");
79+
80+
List<String> lines = Files.readAllLines(Paths.get("./logs/lazy/monitor.log"), StandardCharsets.UTF_8);
81+
int count = 0;
82+
for (String line : lines) {
83+
if (line.contains("LAZY-INIT - test")) {
84+
++count;
85+
}
86+
}
87+
Assert.assertTrue(count >= 4);
88+
}
89+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<Configuration status="OFF">
3+
<Appenders>
4+
<RollingFile name="LAZY-INIT-APPENDER"
5+
fileName="${logging.path.lazy.init}/lazy/monitor.log" append="true"
6+
filePattern="${logging.path.lazy.init}/lazy/monitor.log.%d{${date.pattern.lazy.init}}">
7+
<ThresholdFilter level="${logging.level.lazy.init}" onMatch="ACCEPT" onMismatch="DENY"/>
8+
<PatternLayout charset="UTF-8">
9+
<pattern>%d [%t] %-5p %c{2} - %m%n %throwable</pattern>
10+
</PatternLayout>
11+
<Policies>
12+
<!-- 按天分日志文件:重要的是 filePattern 配置到按照天 -->
13+
<TimeBasedTriggeringPolicy interval="1" modulate="true"/>
14+
</Policies>
15+
</RollingFile>
16+
</Appenders>
17+
18+
<Loggers>
19+
<Logger name="LAZY-INIT" level="${logging.level.lazy.init}" additivity="false">
20+
<AppenderRef ref="LAZY-INIT-APPENDER"/>
21+
</Logger>
22+
</Loggers>
23+
</Configuration>

0 commit comments

Comments
 (0)