Merge pull request eclipse-openj9#16873 from JasonFengJ9/criusysprops

CRIU getRestoreSystemProperites() skips non system property entries
ehrenjulzert · Mar 10, 2023 · bfe3610 · bfe3610
2 parents a5df779 + ed49e99
commit bfe3610
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 13 deletions.
diff --git a/runtime/criusupport/criusupport.cpp b/runtime/criusupport/criusupport.cpp
@@ -938,6 +938,8 @@ Java_org_eclipse_openj9_criu_CRIUSupport_checkpointJVMImpl(JNIEnv *env,
 		}
 
 		if (NULL != vm->checkpointState.restoreArgsList) {
+			/* mark -Xoptionsfile= as consumed */
+			FIND_AND_CONSUME_ARG(vm->checkpointState.restoreArgsList, STARTSWITH_MATCH, VMOPT_XOPTIONSFILE_EQUALS, NULL);
 			bool dontIgnoreUnsupportedRestoreOptions = FIND_AND_CONSUME_ARG(vm->checkpointState.restoreArgsList, EXACT_MATCH, VMOPT_XXIGNOREUNRECOGNIZEDRESTOREOPTIONSENABLE, NULL) < 0;
 
 			if ((FALSE == vmFuncs->checkArgsConsumed(vm, vm->portLibrary, vm->checkpointState.restoreArgsList)) && dontIgnoreUnsupportedRestoreOptions) {

diff --git a/runtime/vm/CRIUHelpers.cpp b/runtime/vm/CRIUHelpers.cpp
@@ -547,15 +547,16 @@ getRestoreSystemProperites(J9VMThread *currentThread)
 
 	if (NULL != vm->checkpointState.restoreArgsList) {
 		JavaVMInitArgs *restorArgs = vm->checkpointState.restoreArgsList->actualVMArgs;
-		 J9CmdLineOption* j9Options = vm->checkpointState.restoreArgsList->j9Options;
-		J9MemoryManagerFunctions * mmfns = vm->memoryManagerFunctions;
+		 J9CmdLineOption *j9Options = vm->checkpointState.restoreArgsList->j9Options;
+		J9MemoryManagerFunctions *mmfns = vm->memoryManagerFunctions;
 		J9InternalVMFunctions *vmFuncs = vm->internalVMFunctions;
 		PORT_ACCESS_FROM_JAVAVM(vm);
 
 		UDATA count = 0;
+		JavaVMOption *restorArgOptions = restorArgs->options;
 
 		for (IDATA i = 0; i < restorArgs->nOptions; ++i) {
-			char * optionString = restorArgs->options[i].optionString;
+			char *optionString = restorArgOptions[i].optionString;
 
 			if (strncmp("-D", optionString, 2) == 0) {
 				count++;
@@ -572,17 +573,17 @@ getRestoreSystemProperites(J9VMThread *currentThread)
 			goto done;
 		}
 
-		for (IDATA i = 0, index = 0; i < restorArgs->nOptions; ++i, ++index) {
-			char * optionString = restorArgs->options[i].optionString;
+		for (IDATA i = 0, index = 0; i < restorArgs->nOptions; ++i) {
+			char *optionString = restorArgOptions[i].optionString;
 
-			if (strncmp("-D", optionString, 2) == 0) {
-				char* propValue = NULL;
-				char* propValueCopy = NULL;
-				char* propNameCopy = NULL;
+			if (0 == strncmp("-D", optionString, 2)) {
+				char *propValue = NULL;
+				char *propValueCopy = NULL;
+				char *propNameCopy = NULL;
 				UDATA propNameLen = 0;
 
 				propValue = strchr(optionString + 2, '=');
-				if (propValue == NULL) {
+				if (NULL == propValue) {
 					propNameLen = strlen(optionString) - 2;
 					propValue = optionString + 2 + propNameLen;
 				} else {
@@ -614,7 +615,7 @@ getRestoreSystemProperites(J9VMThread *currentThread)
 				newArray = POP_OBJECT_IN_SPECIAL_FRAME(currentThread);
 
 				J9JAVAARRAYOFOBJECT_STORE(currentThread, newArray, index, newObject);
-				index++;
+				index += 1;
 
 				PUSH_OBJECT_IN_SPECIAL_FRAME(currentThread, (j9object_t)newArray);
 				newObject = mmfns->j9gc_createJavaLangString(currentThread, (U_8 *)propValueCopy, valueLength, J9_STR_TENURE);
@@ -627,6 +628,7 @@ getRestoreSystemProperites(J9VMThread *currentThread)
 				newArray = POP_OBJECT_IN_SPECIAL_FRAME(currentThread);
 
 				J9JAVAARRAYOFOBJECT_STORE(currentThread, newArray, index, newObject);
+				index += 1;
 
 				j9mem_free_memory(propNameCopy);
 				j9mem_free_memory(propValueCopy);
@@ -645,7 +647,6 @@ getRestoreSystemProperites(J9VMThread *currentThread)
 	}
 
 	return returnProperties;
-
 }
 
 

diff --git a/test/functional/cmdLineTests/criu/criu_nonPortable.xml b/test/functional/cmdLineTests/criu/criu_nonPortable.xml
@@ -276,6 +276,8 @@
     <output type="failure" caseSensitive="yes" regex="no">failed properties test</output>
     <output type="required" caseSensitive="yes" regex="no">Pre-checkpoint</output>
     <output type="success" caseSensitive="yes" regex="no">Post-checkpoint</output>
+    <output type="failure" caseSensitive="yes" regex="no">java.lang.NullPointerException</output>
+    <output type="failure" caseSensitive="yes" regex="no">org.eclipse.openj9.criu.JVMRestoreException</output>
     <output type="failure" caseSensitive="yes" regex="no">CRIU is not enabled</output>
     <output type="failure" caseSensitive="yes" regex="no">Operation not permitted</output>
     <!-- If CRIU can't acquire the original thread IDs, this test will fail. Nothing can be done about this failure. -->
@@ -291,6 +293,8 @@
     <output type="failure" caseSensitive="yes" regex="no">failed properties test</output>
     <output type="required" caseSensitive="yes" regex="no">Pre-checkpoint</output>
     <output type="success" caseSensitive="yes" regex="no">Post-checkpoint</output>
+    <output type="failure" caseSensitive="yes" regex="no">java.lang.NullPointerException</output>
+    <output type="failure" caseSensitive="yes" regex="no">org.eclipse.openj9.criu.JVMRestoreException</output>
     <output type="failure" caseSensitive="yes" regex="no">CRIU is not enabled</output>
     <output type="failure" caseSensitive="yes" regex="no">Operation not permitted</output>
     <!-- If CRIU can't acquire the original thread IDs, this test will fail. Nothing can be done about this failure. -->
@@ -306,6 +310,8 @@
     <output type="failure" caseSensitive="yes" regex="no">failed properties test</output>
     <output type="required" caseSensitive="yes" regex="no">Pre-checkpoint</output>
     <output type="success" caseSensitive="yes" regex="no">Post-checkpoint</output>
+    <output type="failure" caseSensitive="yes" regex="no">java.lang.NullPointerException</output>
+    <output type="failure" caseSensitive="yes" regex="no">org.eclipse.openj9.criu.JVMRestoreException</output>
     <output type="failure" caseSensitive="yes" regex="no">CRIU is not enabled</output>
     <output type="failure" caseSensitive="yes" regex="no">Operation not permitted</output>
     <!-- If CRIU can't acquire the original thread IDs, this test will fail. Nothing can be done about this failure. -->

diff --git a/test/functional/cmdLineTests/criu/src/org/openj9/criu/OptionsFileTest.java b/test/functional/cmdLineTests/criu/src/org/openj9/criu/OptionsFileTest.java
@@ -109,7 +109,7 @@ static void propertiesTest2() {
 	}
 
 	static void propertiesTest3() {
-		String optionsContents = "-Dprop1=val1\n-Dprop2=\\\nval2\n-Dprop3=v \\ \n a \\     \n  l3";
+		String optionsContents = "-Dprop1=val1\n-Dprop2=\\\nval2\n-Dprop3=v \\\n a \\\n  l3";
 		Path optionsFilePath = CRIUTestUtils.createOptionsFile("options", optionsContents);
 
 		Path imagePath = Paths.get("cpData");