Skip to content

Commit

Permalink
8329757: Crash with fatal error: DEBUG MESSAGE: Fast Unlock lock on s…
Browse files Browse the repository at this point in the history
…tack

Reviewed-by: pchilanomate, kvn
  • Loading branch information
xmas92 committed Apr 12, 2024
1 parent ece7d43 commit e45fea5
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/hotspot/share/runtime/deoptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ static void restore_eliminated_locks(JavaThread* thread, GrowableArray<compiledV
#ifndef PRODUCT
bool first = true;
#endif // !PRODUCT
DEBUG_ONLY(GrowableArray<oop> lock_order{0};)
// Start locking from outermost/oldest frame
for (int i = (chunk->length() - 1); i >= 0; i--) {
compiledVFrame* cvf = chunk->at(i);
Expand All @@ -400,6 +401,13 @@ static void restore_eliminated_locks(JavaThread* thread, GrowableArray<compiledV
bool relocked = Deoptimization::relock_objects(thread, monitors, deoptee_thread, deoptee,
exec_mode, realloc_failures);
deoptimized_objects = deoptimized_objects || relocked;
#ifdef ASSERT
if (LockingMode == LM_LIGHTWEIGHT && !realloc_failures) {
for (MonitorInfo* mi : *monitors) {
lock_order.push(mi->owner());
}
}
#endif // ASSERT
#ifndef PRODUCT
if (PrintDeoptimizationDetails) {
ResourceMark rm;
Expand Down Expand Up @@ -431,6 +439,11 @@ static void restore_eliminated_locks(JavaThread* thread, GrowableArray<compiledV
#endif // !PRODUCT
}
}
#ifdef ASSERT
if (LockingMode == LM_LIGHTWEIGHT && !realloc_failures) {
deoptee_thread->lock_stack().verify_consistent_lock_order(lock_order, exec_mode != Deoptimization::Unpack_none);
}
#endif // ASSERT
}

// Deoptimize objects, that is reallocate and relock them, just before they escape through JVMTI.
Expand Down Expand Up @@ -1642,7 +1655,7 @@ bool Deoptimization::relock_objects(JavaThread* thread, GrowableArray<MonitorInf
}
}
}
if (LockingMode == LM_LIGHTWEIGHT && exec_mode == Unpack_none) {
if (LockingMode == LM_LIGHTWEIGHT) {
// We have lost information about the correct state of the lock stack.
// Inflate the locks instead. Enter then inflate to avoid races with
// deflation.
Expand Down
58 changes: 58 additions & 0 deletions src/hotspot/share/runtime/lockStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,19 @@

#include "precompiled.hpp"
#include "memory/allocation.hpp"
#include "oops/markWord.hpp"
#include "oops/oop.inline.hpp"
#include "runtime/globals.hpp"
#include "runtime/lockStack.inline.hpp"
#include "runtime/objectMonitor.inline.hpp"
#include "runtime/safepoint.hpp"
#include "runtime/stackWatermark.hpp"
#include "runtime/stackWatermarkSet.inline.hpp"
#include "runtime/thread.hpp"
#include "utilities/copy.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/growableArray.hpp"
#include "utilities/ostream.hpp"

#include <type_traits>
Expand Down Expand Up @@ -99,6 +103,60 @@ void LockStack::verify(const char* msg) const {
}
#endif

#ifdef ASSERT
void LockStack::verify_consistent_lock_order(GrowableArray<oop>& lock_order, bool leaf_frame) const {
int top_index = to_index(_top);
int lock_index = lock_order.length();

if (!leaf_frame) {
// If the lock_order is not from the leaf frame we must search
// for the top_index which fits with the most recent fast_locked
// objects in the lock stack.
while (lock_index-- > 0) {
const oop obj = lock_order.at(lock_index);
if (contains(obj)) {
for (int index = 0; index < top_index; index++) {
if (_base[index] == obj) {
// Found top index
top_index = index + 1;
break;
}
}

if (VM_Version::supports_recursive_lightweight_locking()) {
// With recursive looks there may be more of the same object
while (lock_index-- > 0 && lock_order.at(lock_index) == obj) {
top_index++;
}
assert(top_index <= to_index(_top), "too many obj in lock_order");
}

break;
}
}

lock_index = lock_order.length();
}

while (lock_index-- > 0) {
const oop obj = lock_order.at(lock_index);
const markWord mark = obj->mark_acquire();
assert(obj->is_locked(), "must be locked");
if (top_index > 0 && obj == _base[top_index - 1]) {
assert(mark.is_fast_locked() || mark.monitor()->is_owner_anonymous(),
"must be fast_locked or inflated by other thread");
top_index--;
} else {
assert(!mark.is_fast_locked(), "must be inflated");
assert(mark.monitor()->owner_raw() == get_thread() ||
(!leaf_frame && get_thread()->current_waiting_monitor() == mark.monitor()),
"must be owned by (or waited on by) thread");
assert(!contains(obj), "must not be on lock_stack");
}
}
}
#endif

void LockStack::print_on(outputStream* st) {
for (int i = to_index(_top); (--i) >= 0;) {
st->print("LockStack[%d]: ", i);
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/share/runtime/lockStack.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
class JavaThread;
class OopClosure;
class outputStream;
template<typename>
class GrowableArray;

class LockStack {
friend class LockStackTest;
Expand Down Expand Up @@ -119,6 +121,9 @@ class LockStack {

// Printing
void print_on(outputStream* st);

// Verify Lock Stack consistent with lock order
void verify_consistent_lock_order(GrowableArray<oop>& lock_order, bool leaf_frame) const NOT_DEBUG_RETURN;
};

#endif // SHARE_RUNTIME_LOCKSTACK_HPP
72 changes: 72 additions & 0 deletions test/hotspot/jtreg/compiler/escapeAnalysis/Test8329757.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright (c) 2024, 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 8329757
* @summary Deoptimization with nested eliminated and not eliminated locks
* caused reordered lock stacks. This can be handled by the interpreter
* but when a frame is migrated back to compiled code via OSR the C2
* assumption about balanced monitorenter-monitorexit is broken.
*
* @requires vm.compMode != "Xint"
*
* @run main/othervm compiler.escapeAnalysis.Test8329757
*/

package compiler.escapeAnalysis;

public class Test8329757 {

int a = 400;
Double ddd;

void q() {
int e;
synchronized (new Double(1.1f)) {
int[] f = new int[a];
synchronized (Test8329757.class) {
for (int d = 4; d < 127; d++) {
e = 13;
do switch (d * 5) {
case 0:
case 42:
case 29:
e = d;
default:
f[1] = e;
} while (--e > 0);
}
}
}
}

void n() {
for (int j = 6; j < 274; ++j) q();
}

public static void main(String[] args) {
Test8329757 r = new Test8329757();
for (int i = 0; i < 1000; i++) r.n();
}
}

5 comments on commit e45fea5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@rkennke
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport lilliput-jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on e45fea5 Apr 22, 2024

Choose a reason for hiding this comment

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

@rkennke Could not automatically backport e45fea5a to openjdk/lilliput-jdk21u due to conflicts in the following files:

  • src/hotspot/share/runtime/deoptimization.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/lilliput-jdk21u. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/lilliput-jdk21u.git lilliput:lilliput

# Check out the target branch and create your own branch to backport
$ git checkout lilliput
$ git checkout -b backport-rkennke-e45fea5a

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git e45fea5a801ac09c3d572ac07d6179e80c422942

# Backport the commit
$ git cherry-pick --no-commit e45fea5a801ac09c3d572ac07d6179e80c422942
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport e45fea5a801ac09c3d572ac07d6179e80c422942'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/lilliput-jdk21u with the title Backport e45fea5a801ac09c3d572ac07d6179e80c422942.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit e45fea5a from the openjdk/jdk repository.

The commit being backported was authored by Axel Boldt-Christmas on 12 Apr 2024 and was reviewed by Patricio Chilano Mateo and Vladimir Kozlov.

Thanks!

@rkennke
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport lilliput-jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on e45fea5 Apr 22, 2024

Choose a reason for hiding this comment

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

@rkennke the backport was successfully created on the branch backport-rkennke-e45fea5a in my personal fork of openjdk/lilliput-jdk21u. To create a pull request with this backport targeting openjdk/lilliput-jdk21u:lilliput, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit e45fea5a from the openjdk/jdk repository.

The commit being backported was authored by Axel Boldt-Christmas on 12 Apr 2024 and was reviewed by Patricio Chilano Mateo and Vladimir Kozlov.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/lilliput-jdk21u:

$ git fetch https://github.com/openjdk-bots/lilliput-jdk21u.git backport-rkennke-e45fea5a:backport-rkennke-e45fea5a
$ git checkout backport-rkennke-e45fea5a
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/lilliput-jdk21u.git backport-rkennke-e45fea5a

Please sign in to comment.