Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MyPerf4J-90] Fix SysGenProfilingFile rename fail #90 #91

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public String legacyKey() {
return legacyKey;
}

@Override
public String toString() {
return key;
}

public static ConfigKey of(String key, String legacyKey) {
return new ConfigKey(key, legacyKey);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -51,8 +55,27 @@ private static void recordMetrics0(int methodTagId, int tp95, int tp99, int tp99
public static void buildSysGenProfilingFile() {
final long startMills = System.currentTimeMillis();
final String filePath = ProfilingConfig.basicConfig().sysProfilingParamsFile();

try {
final File tempFile = buildTmpProfilingFile(filePath);
final Path tempPath = tempFile.toPath();
JiaRG marked this conversation as resolved.
Show resolved Hide resolved
final Path destPath = Files.move(tempPath, Paths.get(filePath), StandardCopyOption.REPLACE_EXISTING);
final File destFile = destPath.toFile();
// 此处不能设置只读,否则在windows环境下次move的时候会报错 AccessDeniedException
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果 destFile.setReadOnly() 之后再次 move 会报错的话,那 FileTest 的第 89 行就会抛异常

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不一样,FileTest 能跑成功,但是 agent 加载到 tomcat 里面就会报错

final boolean rename = destFile.exists()/* && destFile.setReadOnly() */;
Logger.debug("MethodMetricsHistogram.buildSysGenProfilingFile(): rename " + tempFile.getName()
+ " to " + destFile.getName() + " " + (rename ? "success" : "fail"));
} catch (Exception e) {
Logger.error("MethodMetricsHistogram.buildSysGenProfilingFile() error", e);
} finally {
Logger.debug("MethodMetricsHistogram.buildSysGenProfilingFile() finished, cost="
+ (System.currentTimeMillis() - startMills) + "ms");
}
}

private static File buildTmpProfilingFile(String filePath) throws IOException {
final String tempFilePath = filePath + "_tmp";
final File tempFile = new File(tempFilePath);
final File tempFile = Paths.get(tempFilePath).toFile();
try (BufferedWriter fileWriter = new BufferedWriter(new FileWriter(tempFile, false), 8 * 1024)) {
fileWriter.write("#This is a file automatically generated by MyPerf4J, please do not edit!\n");

Expand All @@ -73,23 +96,13 @@ public static void buildSysGenProfilingFile() {

if (!neverInvokedMethods.isEmpty()) {
fileWriter.write("#The following methods have never been invoked!\n");
for (int i = 0; i < neverInvokedMethods.size(); i++) {
final Integer methodId = neverInvokedMethods.get(i);
for (final Integer methodId : neverInvokedMethods) {
writeProfilingInfo(tagMaintainer, fileWriter, methodId, 128);
}
fileWriter.flush();
}

final File destFile = new File(filePath);
final boolean rename = tempFile.renameTo(destFile) && destFile.setReadOnly();
Logger.debug("MethodMetricsHistogram.buildSysGenProfilingFile(): rename " + tempFile.getName()
+ " to " + destFile.getName() + " " + (rename ? "success" : "fail"));
} catch (Exception e) {
Logger.error("MethodMetricsHistogram.buildSysGenProfilingFile()", e);
} finally {
Logger.debug("MethodMetricsHistogram.buildSysGenProfilingFile() finished, cost="
+ (System.currentTimeMillis() - startMills) + "ms");
}
return tempFile;
}

private static void writeProfilingInfo(MethodTagMaintainer tagMaintainer,
Expand Down
138 changes: 138 additions & 0 deletions MyPerf4J-Core/src/test/java/cn/myperf4j/core/FileTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package cn.myperf4j.core;

import cn.myperf4j.base.util.Logger;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Objects;

/**
* 文件测试
*/
public class FileTest {

private static final File baseDir = new File(System.getProperty("user.dir"));

private static final File testDir = new File(baseDir, "temp");

@BeforeClass
public static void prepare() {
getDir(FileTest.class.getSimpleName());
}

@AfterClass
public static void cleanup() {
deleteDir(FileTest.class.getSimpleName());
}

LinShunKang marked this conversation as resolved.
Show resolved Hide resolved
@Test
public void testFileRename() {
String testFile = getDir().getPath() + "/testFileRename";
final Path testFilePath1 = Paths.get(testFile + 1);
final Path testFilePath2 = Paths.get(testFile + 2);
final Path tmpFilePath = Paths.get(testFile + "_tmp");

for (int i = 0; i < 2; i++) {
try (BufferedWriter fileWriter = new BufferedWriter(new FileWriter(tmpFilePath.toFile(), false), 8192)) {
fileWriter.write("#This is a file automatically generated by MyPerf4J, please do not edit!\n");
fileWriter.flush();
// 流还没释放导致
Assert.assertFalse(tmpFilePath.toFile().renameTo(testFilePath1.toFile()));
Copy link
Owner

@LinShunKang LinShunKang Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 MacOS 里这行代码执行失败

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你之前的代码在 MacOS 里能成功吗?

Assert.assertFalse(testFilePath1.toFile().setReadOnly());
} catch (Exception e) {
Logger.error("testFileRename error", e);
}

if (i == 0) {
// 第一次rename能成功 文件存在的情况setReadOnly始终能成功
Assert.assertTrue(tmpFilePath.toFile().renameTo(testFilePath2.toFile()));
Assert.assertTrue(testFilePath2.toFile().setReadOnly());
} else {
// 第一次rename能成功,后续就失败
Assert.assertFalse(tmpFilePath.toFile().renameTo(testFilePath2.toFile()));
Copy link
Owner

@LinShunKang LinShunKang Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 MacOS 里这行代码执行失败

Assert.assertTrue(testFilePath2.toFile().setReadOnly());
}
}
// tmp文件存在
Assert.assertTrue(tmpFilePath.toFile().exists());
// BufferedWriter没释放时rename的文件不存在
Assert.assertFalse(testFilePath1.toFile().exists());
// BufferedWriter释放后rename的文件存在
Assert.assertTrue(testFilePath2.toFile().exists());
}

@Test
public void testFilesMove() throws IOException {
String testFile = getDir().getPath() + "/testFilesMove";
final Path testFilePath = Paths.get(testFile);
final Path tmpFilePath = Paths.get(testFile + "_tmp");
if (!tmpFilePath.toFile().exists()) {
Files.createFile(tmpFilePath);
tmpFilePath.toFile().setReadOnly();
}
Assert.assertFalse(tmpFilePath.toFile().canWrite());
LinShunKang marked this conversation as resolved.
Show resolved Hide resolved

final Path moved = Files.move(tmpFilePath, testFilePath, StandardCopyOption.REPLACE_EXISTING);
moved.toFile().setReadOnly();
Assert.assertFalse(moved.toFile().canWrite());

Files.move(moved, tmpFilePath, StandardCopyOption.REPLACE_EXISTING);
Assert.assertFalse(tmpFilePath.toFile().canWrite());
}

private File getDir() {
return getDir(getName());
}

private String getName() {
JiaRG marked this conversation as resolved.
Show resolved Hide resolved
return getClass().getSimpleName();
}

private static File getDir(String dir) {
File file = new File(testDir, dir);
if (!file.isDirectory()) {
if (!file.mkdirs()) {
throw new IllegalStateException("Can't create dir: " + file);
}
}
return file;
}

private static void deleteDir(String dir) {
deleteDir(testDir, dir);
}

private static void deleteDir(final File parent, String dir) {
final File dirFile = new File(parent, dir);
if (dirFile.isDirectory()) {
for (final File f: Objects.requireNonNull(dirFile.listFiles())) {
if (f.isDirectory()) {
deleteDir(dirFile, f.getName());
continue;
}
LinShunKang marked this conversation as resolved.
Show resolved Hide resolved
if (f.isFile()) {
if (!f.delete()) {
throw new IllegalStateException("Can't delete file: " + f);
}
Logger.info("Delete file: " + f);
}
}
if (!dirFile.delete()) {
throw new IllegalStateException("Can't delete directory: " + dirFile);
}
Logger.info("Delete directory: " + dirFile);
}
testDir.deleteOnExit();
Logger.info("Delete testDir: " + testDir);
}
}
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>8.29</version>
<version>8.40</version>
</dependency>
<dependency>
<groupId>io.netty</groupId>
Expand Down