FWD » Core Development » Runtime Infrastructure

After a client that runs in batch mode finishes its execution it stops with a crash :

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j  com.goldencode.p2j.ui.client.chui.driver.console.ConsoleHelper.terminate()V+0
j  com.goldencode.p2j.ui.client.chui.driver.console.ConsoleDriver.shutdown()V+10
j  com.goldencode.p2j.main.ClientCore.start(Lcom/goldencode/p2j/cfg/BootstrapConfig;Ljava/util/function/Supplier;)V+142
j  com.goldencode.p2j.main.ClientDriver.start(Lcom/goldencode/p2j/cfg/BootstrapConfig;)V+45
j  com.goldencode.p2j.main.CommonDriver.process([Ljava/lang/String;)V+239
j  com.goldencode.p2j.main.ClientDriver.process([Ljava/lang/String;)V+2
j  com.goldencode.p2j.main.ClientDriver.main([Ljava/lang/String;)V+33
v  ~StubRoutines::call_stub

siginfo: EXCEPTION_ACCESS_VIOLATION (0xc0000005), reading address 0x0000000000000000

After the discussion in #9704 , the problem is in terminal_win.c: void resume()
After looking into terminal_linux.c the code from resume() should be executed only if isSuspended

I tried added the same for terminal_win.c and it seems to fix the issue ✅:

=== modified file 'src/native/terminal_win.c'
--- src/native/terminal_win.c   2025-01-29 11:32:39 +0000
+++ src/native/terminal_win.c   2025-03-05 13:32:51 +0000
@@ -998,40 +998,46 @@
  */
 void resume()
 {
-   // Return code
-   jboolean ret = JNI_FALSE;
-
-   // Return to the previous tty mode stored locally by suspend() call
-   resetMode();
-
-   // For redirected output mode do nothing below
-   if (!isOnScreenOutput)
-   {
-      isSuspended = 0;
-      return;
-   }
-
-   // Refresh to restore the screen size that was before suspension
-   ret = SetConsoleScreenBufferSize(hConsoleOut, screenBufferInfoCurrent.dwSize);
-
-   // Restore saved cursor position
-   if (ret)
-   {
-      ret = SetConsoleCursorPosition(hConsoleOut, screenBufferInfoCurrent.dwCursorPosition);
-   }
-   // And cursor visibility
-   if (ret)
-   {
-      ret = SetConsoleCursorInfo(hConsoleOut, &cursorInfoCurrent);
-   }
-
-   // Error encountered during mode resetting
-   if (!ret)
-   {
-      generateException("resume");
-   }
-
-   isSuspended = 0;
+   if (isSuspended)
+   {
+      // Return code
+      jboolean ret = JNI_FALSE;
+
+       // Return to the previous tty mode stored locally by suspend() call
+       resetMode();
+
+       // For redirected output mode do nothing below
+       if (!isOnScreenOutput)
+       {
+          isSuspended = 0;
+          return;
+       }
+
+       // Refresh to restore the screen size that was before suspension
+       ret = SetConsoleScreenBufferSize(hConsoleOut, screenBufferInfoCurrent.dwSize);
+
+
+       // Restore saved cursor position
+       if (ret)
+       {
+          ret = SetConsoleCursorPosition(hConsoleOut, screenBufferInfoCurrent.dwCursorPosition);
+       }
+       // And cursor visibility
+       if (ret)
+       {
+          ret = SetConsoleCursorInfo(hConsoleOut, &cursorInfoCurrent);
+       }
+
+       // Error encountered during mode resetting
+       if (!ret)
+       {
+          generateException("resume");
+       }
+
+       isSuspended = 0;
+
+   }
+
 }

I have some converns with how we are managing the suspend/resume here. These concerns are not just related to the changes proposed here.

1. Do we have thread safety issues in managing this isSuspended flag? The suspend() and resume() are called as part of the child process launching code. What happens if we are launching a child process using NO-WAIT? In that scenario we can have multiple child processes simultaneously. I'm not sure the suspend/resume is designed for that case.

2. Protection of this would probably be better handled in Java code rather than unconditionally downcalling to the native code. There are synchronization primitives that we can use in the native code BUT at the cost of making the native code more complicated. I think any protective code is cleaner and simpler in Java.

Greg Shah wrote:

1. Do we have thread safety issues in managing this isSuspended flag? The suspend() and resume() are called as part of the child process launching code. What happens if we are launching a child process using NO-WAIT? In that scenario we can have multiple child processes simultaneously. I'm not sure the suspend/resume is designed for that case.

A question is: if there are multiple child processes running, does this mean that 'suspend mode' must remain active until the last process exits?

Almost certainly yes.

Reopening the discussion on this.

Is it ok to fix this by adding the if (isSuspended) (#9740-1) that is in the linux version terminal_linux.c and not in the windows equivalent terminal_win.c ?

I don't have much knowledge about this, but making the windows script match the linux version looks ok to me.

Please see if you can test my concern in #9740-2 that there could be multiple suspended terminals simultaneously. If that is the case, the current code is broken.

Greg Shah wrote:

Please see if you can test my concern in #9740-2 that there could be multiple suspended terminals simultaneously. If that is the case, the current code is broken.

I tried something like this and it worked:

os-command no-wait value('start powershell.exe -Command "pause"').
os-command no-wait value(".\1.cmd 1 start1").
os-command no-wait value('start powershell.exe -Command "pause"').

The first and third command would spawn two powershell terminals and leave them suspended, while the second executes successfully .\1.cmd

Is this what you wanted me to test? Or i misunderstood?

Please check that the code client.suspend(silent); on line 276 of ProcessDaemon is executed multuple times before the client.resume(silent); from line 323 is executed once.

What happens if the 2nd or 3rd launch is NOT no-wait? In other words, one or more no-wait launches are running (and paused) and then you run something without no-wait that starts and ends before the other processes finish?

Greg Shah wrote:

Please check that the code client.suspend(silent); on line 276 of ProcessDaemon is executed multuple times before the client.resume(silent); from line 323 is executed once.

I added some counters for suspend and resume.

What happens if the 2nd or 3rd launch is NOT no-wait? In other words, one or more no-wait launches are running (and paused) and then you run something without no-wait that starts and ends before the other processes finish?

It worked as expected (1st and 2nd command started in parallel and the third started after the second one finished its execution)

Test-case :

put  unformatted  "First command " now "~n".
os-command no-wait value('timeout /t 20 /nobreak').

put  unformatted  "Second command " now "~n".
os-command value(".\1.cmd 2 start1").

put  unformatted  "Third command " now "~n".
os-command no-wait value('timeout /t 20 /nobreak').

Output :

First command 03/17/2025 13:18:41.454+02:00
client.suspend counter :1
client.resume counter:1
Second command 03/17/2025 13:18:41.955+02:00
client.suspend counter :2
client.resume counter:2
Third command 03/17/2025 13:18:44.289+02:00
client.suspend counter :3
client.resume counter:3

In the testcase from #9740-10, what happens if the 2nd command runs longer than the 3rd command?

Greg Shah wrote:

In the testcase from #9740-10, what happens if the 2nd command runs longer than the 3rd command?

The second command doesn't have no-wait, so the AVM waits for it to finish its execution before going to the next statement.
The third starts only after the second one finished.

Good point. OK, move ahead with the if (isSuspended) addition on Windows.

Code Review Task Branch 9740a Revision 15780

The change is good.

I think there is a latent bug here: the isSuspended = 0; that is done after the generateException("resume");. If the except is generated, then the isSuspended will not be cleared. That seems wrong because we will never be able to clear it properly. I think the isSuspended = 0; can be moved to just after resetMode();. That would also allow the removal of the isSuspended = 0; on line 1013. The linux code doesn't have this problem.

Greg Shah wrote:

I think there is a latent bug here: the isSuspended = 0; that is done after the generateException("resume");. If the except is generated, then the isSuspended will not be cleared. That seems wrong because we will never be able to clear it properly. I think the isSuspended = 0; can be moved to just after resetMode();. That would also allow the removal of the isSuspended = 0; on line 1013. The linux code doesn't have this problem.

Good point, thanks.

Committed to 9740a / rev 15781

Greg Shah wrote:

Code Review Task Branch 9740a Revision 15781

The change is good.

Please confirm that Windows process launching, in all variations, is working.

What i've tested so far used os-command. Are there other ways of launching a process in 4gl?

Lorian Sandu wrote:

Greg Shah wrote:

Code Review Task Branch 9740a Revision 15781

The change is good.

Please confirm that Windows process launching, in all variations, is working.

What i've tested so far used os-command. Are there other ways of launching a process in 4gl?

Yes. There are other "interactive" mechanisms that are the equivalent to OS-COMMAND: DOS and UNIX are the most common. But these just convert to the same ProcessOps code for launching. I don't think you need to test them since the conversion hasn't changed.

The other process launching (INPUT THROUGH, INPUT-OUTPUT THROUGH and OUTPUT THROUGH) is non-interactive which uses STDIO redirection instead of the terminal manipulation. I think that code is safe since it uses a different version of launch().

If you've covered OS-COMMAND thoroughly, then we are OK.

Greg Shah wrote:

Yes. There are other "interactive" mechanisms that are the equivalent to OS-COMMAND: DOS and UNIX are the most common. But these just convert to the same ProcessOps code for launching. I don't think you need to test them since the conversion hasn't changed.

The other process launching (INPUT THROUGH, INPUT-OUTPUT THROUGH and OUTPUT THROUGH) is non-interactive which uses STDIO redirection instead of the terminal manipulation. I think that code is safe since it uses a different version of launch().

Thanks for the clarifications.

If you've covered OS-COMMAND thoroughly, then we are OK.

I think we can put 9740a in the queue for merge.

It has the same stack trace (and failure) as the original report:

j  com.goldencode.p2j.ui.client.chui.driver.console.ConsoleHelper.terminate()V+0
j  com.goldencode.p2j.ui.client.chui.driver.console.ConsoleDriver.shutdown()V+10
j  com.goldencode.p2j.main.ClientCore.start(Lcom/goldencode/p2j/cfg/BootstrapConfig;Ljava/util/function/Supplier;)V+142
j  com.goldencode.p2j.main.ClientDriver.start(Lcom/goldencode/p2j/cfg/BootstrapConfig;)V+45
j  com.goldencode.p2j.main.CommonDriver.process([Ljava/lang/String;)V+239
j  com.goldencode.p2j.main.ClientDriver.process([Ljava/lang/String;)V+2
j  com.goldencode.p2j.main.ClientDriver.main([Ljava/lang/String;)V+33

Kevin, please double-check that you are not using an old p2j.dll.

I pulled code, cleaned, and refreshed my local environment yesterday morning. The time stamp on the p2j.dll files in my environment reflect that.

• deploy/lib/p2j.dll 03/25/2025 8:15am
• deploy/spawner/p2j.dll 03/25/2025 8:15am

This is strange.

I've tested again on my Windows machine and i don't get the crash.

This is how the client is started (the same as rptrun on your side):

java -Xmx128m -XX:+HeapDumpOnOutOfMemoryError -Djava.library.path=../lib/ -classpath ../lib/p2j.jar;  com.goldencode.p2j.main.ClientDriver net:socket:nio=false net:server:host=localhost net:connection:secure=false net:server:secure_port=3333 net:server:insecure_port=3433  client:mode:batch=true client:driver:background=true client:cmd-line-option:debugalert=true client:cmd-line-option:errorstack=true client:cmd-line-option:startup-procedure=test.p 1>>out.log 2>>err.log

Where does this Redmine stand? It's in 'Test' status, but Kevin can still recreate this.

We can't recreate the problem here. The only theory we have is that the p2j.dll is not updated with a later version.

Lorian: Please move ahead with #9851 which would help us rule out the p2j.dll as the cause.

We need some relief from this bug. All developers can recreate at-will in their local Window's environment. Looks like forward progress has stalled out with cannot recreate/questioning our version of p2j.dll. So I recommend we move forward in one of two ways: Lorian meets with Carson and Carson demo's the crash, or GC provides a p2j.dll for us to drop in and test with.

We'll get you changes that allow the building of the p2j.dll with embedded version info.

Lorian: Please make #9851 a priority.

We will be pulling and building p2j 15932 tonight and with the holiday Monday this will be tested early Tuesday morning.

Thanks for the update. The good news is that the root cause (crash) is resolved when using an up-to-date version of the p2j.dll.

The next step would be to investigate why the p2j.dll is not being built correctly on your side during the FWD build process.

The changes from #9851 that hardcode the FWD version in the native code are ready and should be available to you very soon.

While testing and researching yesterday, I discovered that our job to build Windows Native is encountering a compile error. I am investigating what might be the issue but though I would post the compile errors here in case someone knows a quick cause/resolution. Thanks.

[ant:javac] Compiling 452 source files to C:\Gitlab-Runner\builds\glrt-CaL\0\sxe\sx.nextgen\appcode\p2j\build\classes
[ant:javac] C:\Gitlab-Runner\builds\glrt-CaL\0\sxe\sx.nextgen\appcode\p2j\src\com\goldencode\p2j\email\SmtpEmail.java:1159: error: incompatible types: com.goldencode.p2j.util.osresource.RemoteEmailSender cannot be converted to com.goldencode.p2j.email.RemoteEmailSender
[ant:javac]          return OSResourceManager.getEmailSender();
[ant:javac]                                                 ^
[ant:javac] C:\Gitlab-Runner\builds\glrt-CaL\0\sxe\sx.nextgen\appcode\p2j\src\com\goldencode\p2j\ui\SpreadsheetWidget.java:303: warning: non-varargs call of varargs method with inexact argument type for last parameter;
[ant:javac]       invokeWidgetCommand(SpreadsheetCommands.DESTROY, null);
[ant:javac]                                                        ^
[ant:javac]   cast to Object for a varargs call
[ant:javac]   cast to Object[] for a non-varargs call and to suppress this warning
[ant:javac] C:\Gitlab-Runner\builds\glrt-CaL\0\sxe\sx.nextgen\appcode\p2j\src\com\goldencode\p2j\util\ChUIPrinterStreamSupport.java:131: error: incompatible types: com.goldencode.p2j.util.osresource.Launcher cannot be converted to com.goldencode.p2j.util.Launcher
[ant:javac]          Launcher processLauncher = OSResourceManager.getProcessLauncher();
[ant:javac]                                                                         ^
[ant:javac] C:\Gitlab-Runner\builds\glrt-CaL\0\sxe\sx.nextgen\appcode\p2j\src\com\goldencode\p2j\util\ProcessOps.java:401: error: incompatible types: com.goldencode.p2j.util.osresource.Launcher cannot be converted to com.goldencode.p2j.util.Launcher
[ant:javac]          wa.rl = OSResourceManager.getProcessLauncher();
[ant:javac]                                                      ^
[ant:javac] Note: Some input files use unchecked or unsafe operations.
[ant:javac] Note: Recompile with -Xlint:unchecked for details.
[ant:javac] 3 errors
[ant:javac] 1 warning
> Task :ant-compile FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':ant-compile'.
> Compile failed; see the compiler error output for details.

Hi, I think this was not a clean build (Compiling 452 source files suggests this). Please do a clean (or manually delete p2j\build folder) and try the build again.

We have a clean compile and updated p2j.dll. With this in place, we are no longer receiving thread dumps for each report execution.

Okay to close.