Skip to content

Commit

Permalink
[GR-34669] GC policy sizes lock used before thread object is assigned.
Browse files Browse the repository at this point in the history
PullRequest: graal/10218
  • Loading branch information
peter-hofer committed Nov 6, 2021
2 parents 6127e51 + e579142 commit 1f48ff7
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@
*/
package com.oracle.svm.core.genscavenge;

import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.atomic.AtomicBoolean;

import org.graalvm.compiler.api.replacements.Fold;
import org.graalvm.compiler.nodes.PauseNode;
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.Platforms;
import org.graalvm.word.UnsignedWord;
Expand Down Expand Up @@ -78,7 +79,7 @@ static int getMaxSurvivorSpaces(Integer userValue) {
protected int tenuringThreshold;

protected volatile SizeParameters sizes;
private final ReentrantLock sizesUpdateLock = new ReentrantLock();
private final AtomicBoolean sizesUpdateSpinLock = new AtomicBoolean();

protected AbstractCollectionPolicy(int initialNewRatio, int initialTenuringThreshold) {
this.initialNewRatio = initialNewRatio;
Expand Down Expand Up @@ -145,11 +146,18 @@ public void updateSizeParameters() {
if (previous != null && params.equal(previous)) {
return; // nothing to do
}
sizesUpdateLock.lock();
while (!sizesUpdateSpinLock.compareAndSet(false, true)) {
/*
* We use a primitive spin lock because at this point, the current thread might be
* unable to use a Java lock (e.g. no Thread object yet), and the critical section is
* short, so we do not want to suspend and wake up threads for it.
*/
PauseNode.pause();
}
try {
updateSizeParametersLocked(params, previous);
} finally {
sizesUpdateLock.unlock();
sizesUpdateSpinLock.set(false);
}
guaranteeSizeParametersInitialized(); // sanity
}
Expand Down Expand Up @@ -303,10 +311,9 @@ protected SizeParameters computeSizeParameters(SizeParameters existing) {
/*
* In HotSpot, this is the reserved capacity of each of the survivor From and To spaces,
* i.e., together they occupy 2x this size. Our chunked heap doesn't reserve memory, so
* we use never occupy more than 1x this size for survivors except during collections.
* we never occupy more than 1x this size for survivors except during collections.
* However, this is inconsistent with how we interpret the maximum size of the old
* generation, which we can exceed the (current) old gen size while copying during
* collections.
* generation, which we can exceed while copying during collections.
*/
initialSurvivor = initialYoung.unsignedDivide(AbstractCollectionPolicy.INITIAL_SURVIVOR_RATIO);
initialSurvivor = minSpaceSize(alignDown(initialSurvivor));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public interface pthread_condattr_t extends PointerBase {
@CFunction
public static native int pthread_create(pthread_tPointer newthread, pthread_attr_t attr, WordBase start_routine, WordBase arg);

@CFunction(value = "pthread_create", transition = Transition.NO_TRANSITION)
public static native int pthread_create_no_transition(pthread_tPointer newthread, pthread_attr_t attr, WordBase start_routine, WordBase arg);

@CFunction
public static native int pthread_join(pthread_t th, WordPointer thread_return);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
import org.graalvm.nativeimage.c.constant.CConstant;
import org.graalvm.nativeimage.c.function.CFunction;
import org.graalvm.nativeimage.c.function.CFunction.Transition;
import org.graalvm.nativeimage.c.function.CFunctionPointer;
import org.graalvm.nativeimage.c.struct.CStruct;
import org.graalvm.nativeimage.c.type.CIntPointer;
import org.graalvm.word.PointerBase;
import org.graalvm.word.WordBase;

import com.oracle.svm.core.windows.headers.WinBase.HANDLE;
import com.oracle.svm.core.windows.headers.WinBase.LPHANDLE;
Expand All @@ -51,8 +51,8 @@ public class Process {
public static native int TOKEN_QUERY();

@CFunction
public static native HANDLE _beginthreadex(PointerBase security, int stacksize, CFunctionPointer start_address,
PointerBase arglist, int initflag, CIntPointer thrdaddr);
public static native HANDLE _beginthreadex(PointerBase security, int stacksize, PointerBase start_address,
WordBase arglist, int initflag, CIntPointer thrdaddr);

@CConstant
public static native int CREATE_SUSPENDED();
Expand Down Expand Up @@ -92,6 +92,10 @@ public interface CONDITION_VARIABLE extends PointerBase {
public static native int SleepConditionVariableCS(PCONDITION_VARIABLE cond, PCRITICAL_SECTION mutex, int dwMilliseconds);

public static class NoTransitions {
@CFunction(transition = Transition.NO_TRANSITION)
public static native HANDLE _beginthreadex(PointerBase security, int stacksize, PointerBase start_address,
WordBase arglist, int initflag, CIntPointer thrdaddr);

@CFunction(transition = Transition.NO_TRANSITION)
public static native HANDLE GetCurrentProcess();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,18 @@ public boolean isDaemon() {
@Substitute
@TargetElement(onlyWith = NotLoomJDK.class)
static Thread currentThread() {
return JavaThreads.currentThread.get();
Thread thread = JavaThreads.currentThread.get();
assert thread != null : "Thread has not been set yet";
return thread;
}

@Uninterruptible(reason = "called from uninterruptible code", mayBeInlined = true)
@Substitute
@TargetElement(onlyWith = LoomJDK.class)
private static Thread currentThread0() {
return JavaThreads.currentThread.get();
Thread thread = JavaThreads.currentThread.get();
assert thread != null : "Thread has not been set yet";
return thread;
}

@Inject @TargetElement(onlyWith = LoomJDK.class)//
Expand Down

0 comments on commit 1f48ff7

Please sign in to comment.