On 10/02/2013 01:04 AM, Henry Jen wrote:
Hi,
Please review the updated webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8023524/3/webrev/
This update addressed comments from Mandy with following,
- call doPrivileged with specific file permission, "<<ALL FILES>>", "write".
- Use nio package to write deal with FS, also create directory hierarchy
with package name instead of flat files.
- If not specify a value or empty string, the directory is default to
current working directory as "." is specified.
We continue to use local encoding rule as other suggestions doesn't fit.
The reason we cannot use something like URLEncoder is that ":" won't get
encoded and it's not valid character for filename on Windows. All the
suggestions so far seems to have that same issue.
Cheers,
Henry
Hi Henry,
most of the code that use atomic values is not needed if you do all
these checks
in the static block (because the static block is protected by a lock) and
also instead of injecting the dump_dir, I think it's better to ask for it
in ProxyClassesDumper.
so first, nitpicking!, DUMP_PATH_PROPERTY doesn't need to be a static field,
the code below is equivalent to yours but avoid to create yet another
static field.
// For dumping generated classes to disk, for debugging purposes
/* package */ static final String LAMBDA_PROXY_DUMP_DIR;
static {
String DUMP_PATH_PROPERTY = "jdk.internal.lambda.dumpProxyClasses";
LAMBDA_PROXY_DUMP_DIR= AccessController.doPrivileged(new
GetPropertyAction(DUMP_PATH_PROPERTY));
}
// If requested, dump out to a file for debugging purposes
if (LAMBDA_PROXY_DUMP_DIR != null) {
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
ProxyClassesDumper dumper =
ProxyClassesDumper.getInstance(); // dont send DUMP_DIR here
...
then as I said the code in ProxyClassesDumper can be simplified if instead
of sending the dump directory, the ProxyClassesDumper ask for it.
final class ProxyClassesDumper {
...
private final static ProxyClassesDumper INSTANCE;
static {
// ProxyClassesDumper.getInstance() should never be called
// in the static block of InnerClassLambdaMetafactory
// because it will create a circular dependency (a NPE)
String pathname =
InnerClassLambdaMetafactory.LAMBDA_PROXY_DUMP_DIR.trim();
Path dumpDir = Paths.get(pathname.isEmpty() ? "." : pathname);
dumpDir = validateDumpDir(dumpDir)? dumpDir: null;
INSTANCE = new ProxyClassesDumper(dumpDir);
}
private final Path dumpDir; // null if invalid
// implement dump singleton instance, that's all we need right now
public static ProxyClassesDumper getInstance() {
return INSTANCE;
}
private ProxyClassesDumper(String path) {
this.path = path;
}
private static boolean validateDumpDir(Path path) {
String errMsg; // no need to initialize errMsg here
if (!Files.exists(path)) {
...
public void dumpClass(String className, final byte[] classBytes) {
if (dumpDir == null) {
// invalid dump directory, silently ignore it
return;
}
...
cheers,
Rémi
