Skip to content

Commit

Permalink
Changes in thread interrupts mechanism.
Browse files Browse the repository at this point in the history
1. On Linux when pthread_cond_wait or pthread_cond_timedwait are called, when a signal is deilivered the thread resumes waiting for the condition variable as if it was not interrupted.
   To fix this problem a thread is unparked before sending interrupt to it.
2. When Unsafe.park(boolean isAbsolute, long time) is interrupted on return from this function Thread.isInterrupted() should return true, however this doesn't happen as,
   interrupt flag is cleared in StandardJavaMonitor.monitorWait(long timeoutMilliSeconds). To fix this problem an interrupt flag is set to true on catching InterruptedException exception
   in Unsafe.park(boolean isAbsolute, long time).
3. When thread is being transitioned to not alive state and recieves interrupt signal, it cannot handle it as it is in inconsistent half alive state.
   To fix this problem interrupt signals are blocked for threads being transitioned to not alive state.
4. Between Thread.interrupt() call and sending an interrupt signal to a thread, it can be detached. This case is treated as na error.
   To fix it, when a function sending a signal returns ESRCH, it is not treated as an error.
  • Loading branch information
arodchen committed Aug 18, 2016
1 parent 9a73026 commit 051bd8e
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 8 deletions.
22 changes: 20 additions & 2 deletions com.oracle.max.vm.native/substrate/threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ void *thread_run(void *arg) {
#if log_THREADS
log_println("thread_run: END t=%p", nativeThread);
#endif

setCurrentThreadSignalMaskOnThreadExit(result == 1);

/* Successful thread exit */
return NULL;
}
Expand Down Expand Up @@ -419,6 +422,9 @@ int thread_detachCurrent() {
#endif
return JNI_OK;
}

setCurrentThreadSignalMaskOnThreadExit(false);

threadLocalsBlock_setCurrent(0);
threadLocalsBlock_destroy(tlBlock);
return JNI_OK;
Expand Down Expand Up @@ -474,13 +480,25 @@ Java_com_sun_max_vm_thread_VmThread_nativeInterrupt(JNIEnv *env, jclass c, Addre
// Signals the thread
int result = thr_kill(nativeThread, SIGUSR1);
if (result != 0) {
log_exit(11, "Error sending signal SIGUSR1 to native thread %p", nativeThread);
if (result == ESRCH) {
#if log_MONITORS
log_println("Interrupting thread %p failed, as it is already detached", nativeThread);
#endif
} else {
log_exit(11, "Error sending signal SIGUSR1 to native thread %p", nativeThread);
}
}
#elif os_LINUX || os_DARWIN
// Signals the thread
int result = pthread_kill((pthread_t) nativeThread, SIGUSR1);
if (result != 0) {
log_exit(11, "Error sending signal SIGUSR1 to native thread %p", nativeThread);
if (result == ESRCH) {
#if log_MONITORS
log_println("Interrupting thread %p failed, as it is already detached", nativeThread);
#endif
} else {
log_exit(11, "Error sending signal SIGUSR1 to native thread %p", nativeThread);
}
}
#elif os_MAXVE
maxve_interrupt((void*) nativeThread);
Expand Down
18 changes: 18 additions & 0 deletions com.oracle.max.vm.native/substrate/trap.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ static sigset_t vmSignals;
*/
static sigset_t vmAndDefaultSignals;

/**
* The signals blocked on thread exit.
*/
static sigset_t blockedOnThreadExitSignals;

#endif

int getTrapNumber(int signal) {
Expand All @@ -94,6 +99,15 @@ int getTrapNumber(int signal) {
#define thread_setSignalMask pthread_sigmask
#endif

void setCurrentThreadSignalMaskOnThreadExit(boolean isVmOperationThread) {
#if !os_MAXVE
if (!isVmOperationThread) {
/* disable signals sent by Thread.interrupt() as thread is transitioning to not alive state. */
thread_setSignalMask(SIG_BLOCK, &blockedOnThreadExitSignals, NULL);
}
#endif
}

void setCurrentThreadSignalMask(boolean isVmOperationThread) {
#if !os_MAXVE
if (isVmOperationThread) {
Expand Down Expand Up @@ -470,6 +484,10 @@ void nativeTrapInitialize(Address javaTrapStub) {
/* Let all threads be stopped by a debugger. */
sigaddset(&vmSignals, SIGTRAP);

/* Define the signals to be blocked on thread exit. */
sigemptyset(&blockedOnThreadExitSignals);
sigaddset(&blockedOnThreadExitSignals, SIGUSR1);

/* Apply the normal thread mask to the primordial thread. */
thread_setSignalMask(SIG_BLOCK, &allSignals, NULL);
thread_setSignalMask(SIG_UNBLOCK, &vmSignals, NULL);
Expand Down
5 changes: 5 additions & 0 deletions com.oracle.max.vm.native/substrate/trap.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ extern SignalHandlerFunction userSignalHandler;
*/
extern boolean traceSignals;

/**
* Sets the signal mask for the current thread on thread exit.
*/
extern void setCurrentThreadSignalMaskOnThreadExit(boolean isVmOperationThread);

/**
* Sets the signal mask for the current thread. The signals in the mask are those
* that are blocked for the thread.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ public void park(boolean isAbsolute, long time) {
thread.park();
}
} catch (InterruptedException e) {
// do nothing.
thread.setInterrupted();
}
}
}
41 changes: 36 additions & 5 deletions com.oracle.max.vm/src/com/sun/max/vm/thread/VmThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import com.sun.max.atomic.*;
import com.sun.max.lang.*;
import com.sun.max.memory.*;
import com.sun.max.platform.OS;
import com.sun.max.platform.Platform;
import com.sun.max.unsafe.*;
import com.sun.max.vm.*;
import com.sun.max.vm.actor.holder.*;
Expand All @@ -58,6 +60,7 @@
import com.sun.max.vm.ti.*;
import com.sun.max.vm.value.*;


/**
* The MaxineVM VM specific implementation of threads.
*
Expand Down Expand Up @@ -1424,11 +1427,18 @@ public final void resume0() {
}

public final void interrupt0() {
// Set to true as default. Will be cleared on this VmThread's
// native thread if an InterruptedException is thrown after the
// interruption.
interrupted = true;
if (Platform.platform().os == OS.DARWIN ||
Platform.platform().os == OS.LINUX) {
boolean isInterrupted = interrupt0ByUnparking();
if (isInterrupted) {
return;
}
}
if (!nativeThread.isZero()) {
// Set to true as default. Will be cleared on this VmThread's
// native thread if an InterruptedException is thrown after the
// interruption.
nativeInterrupt(nativeThread);
}
}
Expand Down Expand Up @@ -1525,11 +1535,32 @@ public final void park(long wait) throws InterruptedException {
*/
public final void unpark() {
synchronized (this) {
parkState = 1;
notifyAll();
if (parkState == 2) {
parkState = 1;
notifyAll();
} else {
parkState = 1;
}
}
}

/**
* This method interrupts thread by unparking if it is parked.
*
* @returns true if successfull, false otherwise.
*/
public final boolean interrupt0ByUnparking() {
synchronized (this) {
if (parkState == 2) {
parkState = 1;
notifyAll();
return true;
}
return false;
}
}


public final void pushPrivilegedElement(ClassActor classActor, long frameId, AccessControlContext context) {
privilegedStackTop = new PrivilegedElement(classActor, frameId, context, privilegedStackTop);
}
Expand Down

0 comments on commit 051bd8e

Please sign in to comment.