Skip to content

Commit

Permalink
Initialize recycled continuations in createContinuation before usage
Browse files Browse the repository at this point in the history
Currently, recycled continuations are reset and initialized in
freeContinuation at the end of their use. There are also gaps where
the recycled continuation might retain old values and not be properly
reset.

The above approach leads to corruption and segfaults as seen in
the runJavaThread failure reported in eclipse-openj9#16728.

To fix the above crash, all continuations (new and recycled) are
initialized and reset in createContinuation just before the
continuation begins execution.

Related: eclipse-openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
  • Loading branch information
babsingh committed Dec 19, 2023
1 parent f2a305d commit e46cfd1
Showing 1 changed file with 26 additions and 31 deletions.
57 changes: 26 additions & 31 deletions runtime/vm/ContinuationHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,22 +81,20 @@ createContinuation(J9VMThread *currentThread, j9object_t continuationObject)
vm->avgCacheLookupTime += (I_64)j9time_hires_delta(start, j9time_hires_clock(), OMRPORT_TIME_DELTA_IN_NANOSECONDS);
#endif /* defined(J9VM_PROF_CONTINUATION_ALLOCATION) */

J9JavaStack *stack = NULL;
J9SFJNINativeMethodFrame *frame = NULL;
/* No cache found, allocate new continuation structure. */
if (NULL == continuation) {
#if defined(J9VM_PROF_CONTINUATION_ALLOCATION)
start = j9time_hires_clock();
#endif /* defined(J9VM_PROF_CONTINUATION_ALLOCATION) */
J9JavaStack *stack = NULL;
J9SFJNINativeMethodFrame *frame = NULL;
continuation = (J9VMContinuation*)j9mem_allocate_memory(sizeof(J9VMContinuation), OMRMEM_CATEGORY_THREADS);
if (NULL == continuation) {
vm->internalVMFunctions->setNativeOutOfMemoryError(currentThread, 0, 0);
result = FALSE;
goto end;
}

memset(continuation, 0, sizeof(J9VMContinuation));

#ifdef J9VM_INTERP_GROWABLE_STACKS
#define VMTHR_INITIAL_STACK_SIZE ((vm->initialStackSize > (UDATA) vm->stackSize) ? vm->stackSize : vm->initialStackSize)
#else
Expand All @@ -112,22 +110,6 @@ createContinuation(J9VMThread *currentThread, j9object_t continuationObject)

#undef VMTHR_INITIAL_STACK_SIZE

continuation->stackObject = stack;
continuation->stackOverflowMark2 = J9JAVASTACK_STACKOVERFLOWMARK(stack);
continuation->stackOverflowMark = continuation->stackOverflowMark2;

frame = ((J9SFJNINativeMethodFrame*)stack->end) - 1;
frame->method = NULL;
frame->specialFrameFlags = 0;
frame->savedCP = NULL;
frame->savedPC = (U_8*)(UDATA)J9SF_FRAME_TYPE_END_OF_STACK;
frame->savedA0 = (UDATA*)(UDATA)J9SF_A0_INVISIBLE_TAG;
continuation->sp = (UDATA*)frame;
continuation->literals = NULL;
continuation->pc = (U_8*)J9SF_FRAME_TYPE_JNI_NATIVE_METHOD;
continuation->arg0EA = (UDATA*)&frame->savedA0;
continuation->stackObject->isVirtual = TRUE;

#if defined(J9VM_PROF_CONTINUATION_ALLOCATION)
I_64 totalTime = (I_64)j9time_hires_delta(start, j9time_hires_clock(), OMRPORT_TIME_DELTA_IN_NANOSECONDS);
if (totalTime > 10000) {
Expand All @@ -138,8 +120,32 @@ createContinuation(J9VMThread *currentThread, j9object_t continuationObject)
vm->fastAllocAvgTime += totalTime;
}
#endif /* defined(J9VM_PROF_CONTINUATION_ALLOCATION) */
} else {
/* Reset and reuse the stack in the recycled continuation. */
stack = continuation->stackObject;
stack->previous = NULL;
stack->firstReferenceFrame = 0;
}

/* Reset all fields in the new or recycled continuation. */
memset(continuation, 0, sizeof(J9VMContinuation));

continuation->stackObject = stack;
continuation->stackOverflowMark2 = J9JAVASTACK_STACKOVERFLOWMARK(stack);
continuation->stackOverflowMark = continuation->stackOverflowMark2;

frame = ((J9SFJNINativeMethodFrame*)stack->end) - 1;
frame->method = NULL;
frame->specialFrameFlags = 0;
frame->savedCP = NULL;
frame->savedPC = (U_8*)(UDATA)J9SF_FRAME_TYPE_END_OF_STACK;
frame->savedA0 = (UDATA*)(UDATA)J9SF_A0_INVISIBLE_TAG;
continuation->sp = (UDATA*)frame;
continuation->literals = NULL;
continuation->pc = (U_8*)J9SF_FRAME_TYPE_JNI_NATIVE_METHOD;
continuation->arg0EA = (UDATA*)&frame->savedA0;
continuation->stackObject->isVirtual = TRUE;

J9VMJDKINTERNALVMCONTINUATION_SET_VMREF(currentThread, continuationObject, continuation);

/* GC Hook to register Continuation object. */
Expand Down Expand Up @@ -357,17 +363,6 @@ recycleContinuation(J9JavaVM *vm, J9VMThread *vmThread, J9VMContinuation* contin
{
PORT_ACCESS_FROM_JAVAVM(vm);
bool cached = false;
/* Prepare continuationState for recycle, and reset stack initial state. */
J9SFJNINativeMethodFrame *frame = ((J9SFJNINativeMethodFrame*)continuation->stackObject->end) - 1;
frame->method = NULL;
frame->specialFrameFlags = 0;
frame->savedCP = NULL;
frame->savedPC = (U_8*)(UDATA)J9SF_FRAME_TYPE_END_OF_STACK;
frame->savedA0 = (UDATA*)(UDATA)J9SF_A0_INVISIBLE_TAG;
continuation->sp = (UDATA*)frame;
continuation->literals = NULL;
continuation->pc = (U_8*)J9SF_FRAME_TYPE_JNI_NATIVE_METHOD;
continuation->arg0EA = (UDATA*)&frame->savedA0;

if (!skipLocalCache && (0 < vm->continuationT1Size)) {
/* If called by carrier thread (not global), try to store in local cache first.
Expand Down

0 comments on commit e46cfd1

Please sign in to comment.