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

JDK-8276422 Add command-line option to disable finalization #6442

Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4117378
Hacked-up prototype for --finalization=enabled/disabled option; shoul…
stuart-marks Nov 3, 2021
f446afb
Move --finalization option processing and state into JVM.
stuart-marks Nov 9, 2021
04499e8
Minor cleanups.
stuart-marks Nov 9, 2021
542e4dd
Move finalization disabling into ClassFileParser.
stuart-marks Nov 11, 2021
023e0f4
Merge branch 'master' into add-finalization-disable-option
stuart-marks Nov 11, 2021
2dc16e3
Removed global FinalizationEnabled flag and replaced with InstanceKla…
dholmes-ora Nov 11, 2021
27a07f7
Update test to include checking for presence of Finalizer thread.
stuart-marks Nov 13, 2021
383b4a1
Test that no jdk.FinalizationStatistics events are generated when fin…
Nov 17, 2021
814d33e
Add @bug line to JFR finalization event test.
stuart-marks Nov 17, 2021
27fb90d
Add test for invalid finalization option syntax or value.
stuart-marks Nov 17, 2021
ce9d284
Rename invalid finalization option test.
stuart-marks Nov 17, 2021
69aa626
Renaming within the test class itself.
stuart-marks Nov 17, 2021
01231d0
Revert extraneous whitespace change to globals.hpp.
stuart-marks Nov 18, 2021
71d91de
Change InvalidFinalizationOption test to driver mode.
stuart-marks Nov 18, 2021
bdbb7c6
Simplify InvalidFinalizationOption test.
stuart-marks Nov 18, 2021
9f45e51
Merge branch 'master' into JDK-8276422-disable-finalization-option
stuart-marks Nov 18, 2021
3836cc9
extraneous newline
stuart-marks Nov 18, 2021
911af0b
Include instanceKlass.hpp in arguments.cpp
stuart-marks Nov 18, 2021
5df8bf9
Rename InstanceKlass::finalization_enabled to is_finalization_enabled…
stuart-marks Nov 18, 2021
e357eee
Remove Finalizer.Holder class.
stuart-marks Nov 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -147,6 +147,7 @@ JVM_IsArrayClass
JVM_IsCDSDumpingEnabled
JVM_IsConstructorIx
JVM_IsDumpingClassList
JVM_IsFinalizationEnabled
JVM_IsHiddenClass
JVM_IsInterface
JVM_IsPrimitiveClass
@@ -2835,7 +2835,8 @@ Method* ClassFileParser::parse_method(const ClassFileStream* const cfs,
annotation_default_length,
CHECK_NULL);

if (name == vmSymbols::finalize_method_name() &&
if (InstanceKlass::is_finalization_enabled() &&
name == vmSymbols::finalize_method_name() &&
signature == vmSymbols::void_method_signature()) {
if (m->is_empty_method()) {
_has_empty_finalizer = true;
@@ -4171,7 +4172,8 @@ void ClassFileParser::set_precomputed_flags(InstanceKlass* ik) {
bool f = false;
const Method* const m = ik->lookup_method(vmSymbols::finalize_method_name(),
vmSymbols::void_method_signature());
if (m != NULL && !m->is_empty_method()) {
if (InstanceKlass::is_finalization_enabled() &&
(m != NULL) && !m->is_empty_method()) {
f = true;
}

@@ -759,6 +759,9 @@ JVM_SupportsCX8(void);
JNIEXPORT void JNICALL
JVM_ReportFinalizationComplete(JNIEnv *env, jobject finalizee);

JNIEXPORT jboolean JNICALL
JVM_IsFinalizationEnabled(JNIEnv *env);

/*************************************************************************
PART 2: Support for the Verifier and Class File Format Checker
************************************************************************/
@@ -141,6 +141,7 @@

#endif // ndef DTRACE_ENABLED

bool InstanceKlass::_finalization_enabled = true;

static inline bool is_class_loader(const Symbol* class_name,
const ClassFileParser& parser) {
@@ -329,7 +329,17 @@ class InstanceKlass: public Klass {

static bool _disable_method_binary_search;

// Controls finalizer registration
static bool _finalization_enabled;

public:

// Queries finalization state
static bool is_finalization_enabled() { return _finalization_enabled; }

// Sets finalization state
static void set_finalization_enabled(bool val) { _finalization_enabled = val; }

// The three BUILTIN class loader types
bool is_shared_boot_class() const {
return (_misc_flags & _misc_is_shared_boot_class) != 0;
@@ -690,6 +690,10 @@ JVM_ENTRY(void, JVM_ReportFinalizationComplete(JNIEnv * env, jobject finalizee))
MANAGEMENT_ONLY(FinalizerService::on_complete(JNIHandles::resolve_non_null(finalizee), THREAD);)
JVM_END

JVM_ENTRY(jboolean, JVM_IsFinalizationEnabled(JNIEnv * env))
return InstanceKlass::is_finalization_enabled();
JVM_END

// java.io.File ///////////////////////////////////////////////////////////////

JVM_LEAF(char*, JVM_NativePath(char* path))
@@ -40,6 +40,7 @@
#include "logging/logStream.hpp"
#include "logging/logTag.hpp"
#include "memory/allocation.inline.hpp"
#include "oops/instanceKlass.hpp"
#include "oops/oop.inline.hpp"
#include "prims/jvmtiExport.hpp"
#include "runtime/arguments.hpp"
@@ -2866,6 +2867,17 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
if (FLAG_SET_CMDLINE(ErrorFileToStdout, true) != JVMFlag::SUCCESS) {
return JNI_EINVAL;
}
} else if (match_option(option, "--finalization=", &tail)) {
if (strcmp(tail, "enabled") == 0) {
InstanceKlass::set_finalization_enabled(true);
} else if (strcmp(tail, "disabled") == 0) {
InstanceKlass::set_finalization_enabled(false);
} else {
jio_fprintf(defaultStream::error_stream(),
"Invalid finalization value '%s', must be 'disabled' or 'enabled'.\n",
tail);
return JNI_EINVAL;
}
} else if (match_option(option, "-XX:+ExtendedDTraceProbes")) {
#if defined(DTRACE_ENABLED)
if (FLAG_SET_CMDLINE(ExtendedDTraceProbes, true) != JVMFlag::SUCCESS) {
@@ -61,9 +61,19 @@ static ReferenceQueue<Object> getQueue() {
return queue;
}

static class Holder {
static final boolean ENABLED = isFinalizationEnabled();
}

private static native boolean isFinalizationEnabled();

/* Invoked by VM */
static void register(Object finalizee) {
new Finalizer(finalizee);
if (Holder.ENABLED) {
new Finalizer(finalizee);
} else {
throw new InternalError("unexpected call to Finalizer::register when finalization is disabled");
}
}

private void runFinalizer(JavaLangAccess jla) {
@@ -130,7 +140,7 @@ public Void run() {

/* Called by Runtime.runFinalization() */
static void runFinalization() {
if (VM.initLevel() == 0) {
if (VM.initLevel() == 0 || ! Holder.ENABLED) {
return;
}

@@ -182,14 +192,16 @@ public void run() {
}

static {
ThreadGroup tg = Thread.currentThread().getThreadGroup();
for (ThreadGroup tgn = tg;
tgn != null;
tg = tgn, tgn = tg.getParent());
Thread finalizer = new FinalizerThread(tg);
finalizer.setPriority(Thread.MAX_PRIORITY - 2);
finalizer.setDaemon(true);
finalizer.start();
if (Holder.ENABLED) {
Copy link
Member

@jaikiran jaikiran Nov 18, 2021

Choose a reason for hiding this comment

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

Hello Stuart,
My understanding of the the lazy Holder is that it's there to delay the static initialization of the code that's part of the Holder. In this case here, the Holder is being used right within the static block of the Finalizer class, that too as the first thing. In this case, is that Holder class necessary?

Copy link
Member Author

@stuart-marks stuart-marks Nov 18, 2021

Choose a reason for hiding this comment

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

Huh, good catch! This was mostly left over from an earlier version of the flag that used system properties, which aren't initialized until after the Finalizer class is initialized.

It might be the case that the Holder can be removed at this point, since the finalization-enabled bit is no longer in a system property and is in a native class member that should be available before the VM is started.

I say "might" though because this occurs early in system startup, and weird things potentially happen. For example, suppose the first object with a finalizer is created before the Finalizer class is initialized. The VM will perform an upcall to Finalizer::register. An ordinary call to a static method will ensure the class is initialized before proceeding with the call, but this VM upcall is a special case.... I'll have to investigate this some more.

Copy link
Member

@dholmes-ora dholmes-ora Nov 18, 2021

Choose a reason for hiding this comment

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

@stuart-marks not sure I see how anything is different here compared to the existing logic. The Finalizer class is explicitly initialized quite early in the init process, but if a preceding class's initialization created an object with a finalizer then that same upcall would be involved.

Copy link
Member

@shipilev shipilev Nov 18, 2021

Choose a reason for hiding this comment

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

Do we even have to have a flag on Java side? It looks like these calls are only done as the upcalls from VM, so we might just keep the flag on VM side?

Copy link
Member

@dholmes-ora dholmes-ora Nov 18, 2021

Choose a reason for hiding this comment

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

@shipilev not sure what you mean by "a flag on the Java side". The Java code just queries the VM for the finalization enabled/disabled state and uses that to control things.

Copy link
Contributor

@plevart plevart Nov 18, 2021

Choose a reason for hiding this comment

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

If you then need this "flag" in the assert of registerFinalizer and runFinalization, you could use unsafe.shouldBeInitialized(Finalizer.FinalizerThread.class) as a means to find out whether the flag was set or not...

Copy link
Member Author

@stuart-marks stuart-marks Nov 18, 2021

Choose a reason for hiding this comment

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

The disable-finalization feature is a bit more than experimental. The goal is to provide a faithful representation of what the system will look like when finalization is removed. Of course most of that is objects' finalize methods not being called, but it also includes having no finalizer thread running, as well as having runFinalization (a public API) do nothing at all. Thus I think it's useful to have the flag visible to Java.

Copy link
Member Author

@stuart-marks stuart-marks Nov 18, 2021

Choose a reason for hiding this comment

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

@dholmes-ora If the Finalizer class is initialized explicitly and at the right time, then maybe we can do away with the Holder class entirely. Can you point me to where this is done?

EDIT: looks like here. This seems early enough. I'll try just pulling out the Holder to see if everything works ok.

Copy link
Member Author

@stuart-marks stuart-marks Nov 19, 2021

Choose a reason for hiding this comment

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

I pushed an update to remove the Holder class. It seems to continue to work fine. Thanks for pointing this out @jaikiran !

Copy link
Member

@jaikiran jaikiran Nov 19, 2021

Choose a reason for hiding this comment

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

Thank you Stuart, this changed version looks fine to me.

ThreadGroup tg = Thread.currentThread().getThreadGroup();
for (ThreadGroup tgn = tg;
tgn != null;
tg = tgn, tgn = tg.getParent());
Thread finalizer = new FinalizerThread(tg);
finalizer.setPriority(Thread.MAX_PRIORITY - 2);
finalizer.setDaemon(true);
finalizer.start();
}
}

}
@@ -194,7 +194,10 @@ java.launcher.X.usage=\n\
\ override or augment a module with classes and resources\n\
\ in JAR files or directories.\n\
\ --source <version>\n\
\ set the version of the source in source-file mode.\n\n\
\ set the version of the source in source-file mode.\n\
\ --finalization=<value>\n\
\ controls finalization\n\
\ <value> is one of "enabled" or "disabled"\n\n\
These extra options are subject to change without notice.\n

# Translators please note do not translate the options themselves
@@ -32,4 +32,7 @@ Java_java_lang_ref_Finalizer_reportComplete(JNIEnv* env, jclass cls, jobject fin
JVM_ReportFinalizationComplete(env, finalizee);
}


JNIEXPORT jboolean JNICALL
Java_java_lang_ref_Finalizer_isFinalizationEnabled(JNIEnv* env, jclass cls) {
return JVM_IsFinalizationEnabled(env);
}
@@ -0,0 +1,122 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8276422
* @summary add command-line option to disable finalization
* @run main/othervm FinalizationOption yes
* @run main/othervm --finalization=enabled FinalizationOption yes
* @run main/othervm --finalization=disabled FinalizationOption no
*/
public class FinalizationOption {
static volatile boolean finalizerWasCalled = false;

@SuppressWarnings("deprecation")
protected void finalize() {
finalizerWasCalled = true;
}

static void create() {
new FinalizationOption();
}

/**
* Checks whether the finalizer thread is or is not running. The finalizer thread
* is a thread in the root thread group whose named is "Finalizer".
* @param expected boolean indicating whether a finalizer thread should exist
* @return boolean indicating whether the expectation was met
*/
static boolean checkFinalizerThread(boolean expected) {
ThreadGroup root = Thread.currentThread().getThreadGroup();
for (ThreadGroup parent = root;
parent != null;
root = parent, parent = root.getParent())
;

int nt = 100;
Thread[] threads;
while (true) {
threads = new Thread[nt];
nt = root.enumerate(threads);
if (nt < threads.length)
break;
threads = new Thread[nt + 100];
}

Thread ft = null;
for (int i = 0; i < nt; i++) {
if ("Finalizer".equals(threads[i].getName())) {
ft = threads[i];
break;
}
}

String msg = (ft == null) ? "(none)" : ft.toString();
boolean passed = (ft != null) == expected;
System.out.printf("Finalizer thread. Expected: %s Actual: %s %s%n",
expected, msg, passed ? "Passed." : "FAILED!");
return passed;
}

/**
* Checks whether there was a call to the finalize() method.
* @param expected boolean whether finalize() should be called
* @return boolean indicating whether the expecation was met
*/
static boolean checkFinalizerCalled(boolean expected) {
create();
for (int i = 0; i < 100; i++) {
System.gc();
try {
Thread.sleep(10L);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
}
if (finalizerWasCalled) {
break;
}
}
boolean passed = (expected == finalizerWasCalled);
System.out.printf("Call to finalize(). Expected: %s Actual: %s %s%n",
expected, finalizerWasCalled,
passed ? "Passed." : "FAILED!");
return passed;
}

public static void main(String[] args) {
boolean finalizationEnabled = switch (args[0]) {
case "yes" -> true;
case "no" -> false;
default -> {
throw new AssertionError("usage: FinalizationOption yes|no");
}
};

boolean threadPass = checkFinalizerThread(finalizationEnabled);
boolean calledPass = checkFinalizerCalled(finalizationEnabled);

if (!threadPass || !calledPass)
throw new AssertionError("Test failed.");
}
}
@@ -0,0 +1,52 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8276422
* @summary Invalid/missing values for the finalization option should be rejected
* @library /test/lib
* @run driver InvalidFinalizationOption
*/

import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;

public class InvalidFinalizationOption {
public static void main(String[] args) throws Exception {
record TestData(String arg, String expected) { }

TestData[] testData = {
new TestData("--finalization", "Unrecognized option"),
new TestData("--finalization=", "Invalid finalization value"),
new TestData("--finalization=azerty", "Invalid finalization value")
};

for (var data : testData) {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(data.arg);
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldContain(data.expected);
output.shouldHaveExitValue(1);
}
}
}