FWD » Core Development » Runtime Infrastructure

When clientConfig/outputToFile is specified in the directory for an appserver process, the value is treated as a specification which can have multiple placeholder values. One of the placeholds is %pid%.

The -O <filename> is passed on the spawner command line and is honored in the spawner. Search spawn.c for output_file_name or winspawn.c for szOutputFile to see the code.

This spawner code replaces the %pid% placeholder with the spawner's pid. What we really want is the Java (FWD client's) pid in that filename. Using the spawner's pid is confusing. It is the parent process for the Java client, but that means that the admin must find the OS parent/child process relationship to determine which FWD client is associated with the log. Not good.

I'm trying to understand why the Java client can't handle this redirection itself. We already do something similar with the clientLog 4GL command line feature. Why not standardize this and defer the redirection until after the Java client starts? That would allow us to put the proper pid in. It would also eliminate the #5699 issue because the Java code for replacing the %pid% placeholder will allow us to drop the broken Windows spawner code.

This is how I read the logic for the spawner (I'll be going with the Unix example):

ClientBuilder#localStart() builds a new process to run the spawn command, i.e. the spawner process. The spawner process then creates a child process (1) and exits. The child process (1) sets some ids, opens a pseudo terminal master and slave, forks its own child process (2), closes the pty slave (converting itself to a pure pty master) and waits for the child status. Child process (2) closes pty master (converting itself to a pty slave), redirects stdin/stdout to pty slave and conditionally overwrites that statement with output redirect to a file if the arg is provided with the spawn cmd. At that stage the pid placeholder is replaced - in the pty slave process with its (child process 2) pid.

I still need to go through the code for the Windows spawner.

Now since this logic is related to batch processes, running in background where no client driver seems to be instantiated in the Java code, what do we refer to as Java client if not the pty slave process?

Galya Bogdanova wrote:

... running in background where no client driver seems to be instantiated in the Java code, what do we refer to as Java client if not the pty slave process?

The spawner will use NativeSecureConnection.command to determine the launch command for the Java FWD client for this batch process, and will execute that command. So there is a FWD Java client for the batch process.

The test code doesn't help me understand the process tree/pid list/redirected output log name for the FWD client. Can you please use ps and add logging to the C and Java code as needed to record an example of the various processes and how the logfile name is created and post it here?

The approach seems reasonable.

Eugenie: Do you have any thoughts, questions or concerns?

Greg Shah wrote:

The approach seems reasonable.

Eugenie: Do you have any thoughts, questions or concerns?

I need to see the proposed code changes for winspawn.c to make conclusion. Also we will have to try the change to ensure there are no regressions in Windows spawner.

From Galya, via email:

I haven't looked at the Java classes instantiated by the spawner, so let me ask if there is any reason why we cannot redirect output in Java clients themselves and remove the C logic for output files? I can think of several issues, but there might be something else as well. It will require to identify each client class (or if there is a base one) that can be passed to the spawner in the command built by ClientBuilder and then redirect System.out and System.err in its main method. First of all that would mean changes to more places in the code. Also I don't know if the Java process will have the same permissions to access the workingDir as the spawner. The code for getting pid cross-platform in Java 8 relies on reflection, not ideal and will probably have to be changed with future update of the JDK.

It is a reasonable question. For the 4GL compatibility, when we launch child processes (for example, to implement the 4GL OS-COMMAND statement) we know that the Java process builder support does not give us suitable tools to setup the process state that will work. The redirections are a big part of this (the parent process often must have write access to the STDIN and read access to the combined STDOUT/STDERR). Just as important is the pseudo-terminal state, signal processing and other OS level configuration. Although Java has improved process launching in recent versions (after Java 8), I believe it still doesn't provide the capabilities that we require.

On the other hand, the scenario you are talking about does not have these same requirements. The parent process is the spawner and I don't think we need continued access to the redirected streams. On the other hand, we very much do need control over the working dir, environment, permissions and other process launching state. More importantly, the spawner process is needed for the core of the processing due to the requirement of using the OS-level APIs to start the child process with the correct OS account. I don't think we can do that directly in Java. If you find otherwise, we can consider it.

My proposal cannot actually work for Windows where CreateProcess creates an independent new process without any references. Explained here:

The CreateProcess function creates a new process, which runs independently of the creating process. However, for simplicity, the relationship is referred to as a parent-child relationship.

There is no way to modify the handlers of the child from the parent process.
Basically we set some startup details and a startup command and let it go.

Some compile C with Cygwin where you can simulate fork() and this way get access to the new process (how it's done for the Linux spawner where things work). But I saw here that we don't use Cygwin runtime and I'm not sure if it works if only used compile time.

I'm thinking of only two possible solutions (still not explored):
1. Do it in Java - only for that specific case of a spawned background process, where outputToFile is defined. Then redirect System.out in the ClientDriver like we do with System.err in debug.
2. Run the cmd in the spawner process itself. This looks like a more dangerous exercise, since I don't fully understand the win spawn process. But I can work on it.

Any thoughts on these?

Run the cmd in the spawner process itself. This looks like a more dangerous exercise, since I don't fully understand the win spawn process. But I can work on it.

I don't think we can do this. The spawner process is privileged and the executed code must not have elevated privileges.

I need an ABL procedure that outputs to outputToFile to test it out.

I think you can try any 4GL code which uses DISPLAY, PUT or similar output statement without otherwise specifying the target stream.

Something as simple as DISPLAY "hello". will probably do it. You should also check a child process that generates output to stdout (e.g. OS-COMMAND echo "child-process hello".).

Greg Shah wrote:

I think you can try any 4GL code which uses DISPLAY, PUT or similar output statement without otherwise specifying the target stream.

Something as simple as DISPLAY "hello". will probably do it. You should also check a child process that generates output to stdout (e.g. OS-COMMAND echo "child-process hello".).

I have exactly that - a simple procedure with DISPLAY, but I've never seen anything in that log file. That is with the new testcases scheduled appserver_process and p2j-entry properly configured. There is no issue in any server / client logs. The file is created. It's just empty. Not sure what I'm missing.
Also the server log shows (AppServerLauncher:INFO) Appserver 'app_server' was started successfully (4945ms)!
P.S. I also just tested OS-COMMAND echo "child-process hello". and still got nothing in the log file.

I have another idea of how to solve the pid issue on Windows.

The Windows spawner creates the new process with all the needed attributes as it is now, but the cmd is executing another C compiled file. That's where we set a stdout handler to the properly renamed filename and execute the cmd to spawn JVM received as an argument in the same process.

It will not add much complexity and will be a change only for Windows.

Do you think it's acceptable?

Galya Bogdanova wrote:

That's where we set a stdout handler to the properly renamed filename and execute the cmd to spawn JVM received as an argument in the same process.

Actually it doesn't even have to set any handlers. Just rename the file and execute the java cmd with > outputToFile

Galya Bogdanova wrote:

The Windows spawner creates the new process with all the needed attributes as it is now, but the cmd is executing another C compiled file. That's where we set a stdout handler to the properly renamed filename and execute the cmd to spawn JVM received as an argument in the same process.

This won't work either. I thought system() in Windows can replicate the behavior of execvp() in Unix, but nothing similar is possible, so the java cmd will always be launched in a separate process and its pid will be available only to the Java app.

Please focus on #5703 first and pause work on this task.

I'm not working on this task at the moment, but I randomly got an idea and just want to document it here, since we might need to keep this output file (in Progress it's a thing for batch). It might be easier to find the right log by process create datetime instead of pid of the spawner, so changing the placeholder (or adding a new) might help for Windows.

Eugenie: Please review.

Greg Shah wrote:

Eugenie: Please review.

The change looks good for me.

Starting appserver as scheduled batch initializes ThinClient, BatchDriver, OutputManager, ErrorWriterBatch, calls StandardServer#standardEntry() where AgentPool and Agent are started to listen to commands and then the client receives false as response from the server and quits. The client never uses BatchPrimitives to update the screen and write to stdout, that's why handles as DISPLAY or MESSAGE VIEW AS ALERT-BOX have no output in the file in appserver mode.

Is this the expected behavior? I've asked about it before. The file outputToFile in appserver mode is empty. I think I've seen only messages generated in the client (not from converted procedures) to be written to it. Is there someone who actually uses it successfully?

Galya, I've tested in OpenEdge and only MESSAGE statement (with or without VIEW-AS ALERT-BOX) end up in the agent log. I can't make the DISPLAY or PUT SCREEN emit to the log.

In the FWD Client, the MESSAGE statement ends up in Window.message - what is FWD doing in this code, if is reached, in the Agent case?

Galya, I've tested in OpenEdge and only MESSAGE statement (with or without VIEW-AS ALERT-BOX) end up in the agent log. I can't make the DISPLAY or PUT SCREEN emit to the log.

The agent log still gets all the "internal" (4GL runtime generated logging events) log-manager stuff right? How do we handle that today? I recall we didn't handle this yet.

Greg Shah wrote:

Galya, I've tested in OpenEdge and only MESSAGE statement (with or without VIEW-AS ALERT-BOX) end up in the agent log. I can't make the DISPLAY or PUT SCREEN emit to the log.

The agent log still gets all the "internal" (4GL runtime generated logging events) log-manager stuff right?

You mean error messages or such? Yes, these end up in the agent log in OpenEdge and in FWD, too, if I'm recalling right.

If you mean LOG-MANAGER:WRITE-MESSAGE, in OpenEdge this ends up in the default agent log, if LOGFILE-NAME is not set.

I meant both cases. It sounds like we are OK there, if I understand your points.

Do we have the same control over the log filename for these sessions as is possible in OE? I recall we had some limitations on our agent logging, but I don't recall what those limitations are (or were).

Greg, Constantin, I'll need some clarifications here:
1. What do you refer to as agent logs in FWD? outputToFile is the only one I know of.
2. How do you test (run) appserver agents in the OpenEdge env? Can you give me some cmds to run.
3. Can you confirm outputToFile logs output as expected?

It's fine if you don't have the answer to 3. since that is what we're discussing here.
Also let's get LOG-MANAGER out of the picture for now. In the discussed cases let's have it disabled and let's try to reproduce 4GL behavior.

I haven't tested appserver runs in OpenEdge, so there might be a difference in the agent output compared to batch output, but nevertheless something needs to go out to this outputToFile file in our implementation and it's not. I can't see it used in the FWD code, nor in my test runs.

I'm explain myself better - there is a difference in FWD in the output between batch executed as a standalone client from command line and as scheduled service (aka appserver_process). outputToFile is a directory.xml configuration that applies only to batch running as a scheduled service. As far as I understand it outputToFile is expected to replace the stdout redirect from running batch cmd line like that > output.txt. If not can you point me to some documentation to understand better what we're reproducing with outputToFile.

Standalone running batch works as expected, similar to what we have in 4GL - any MESSAGE or DISPLAY statement is saved to the file.

As I've said already I'm not seeing this behavior with appserver service (scheduled batch) and it's clear from the code as well when debugging that nothing is going back from the agent to the client process to be output.

Please give me more details.

Galya Bogdanova wrote:

Greg, Constantin, I'll need some clarifications here:
1. What do you refer to as agent logs in FWD? outputToFile is the only one I know of.

outputToFile is for STDOUT, which is the equivalent for OpenEdge's Agent default log or batch program's STDOUT. FWD also has a STDERR log.

2. How do you test (run) appserver agents in the OpenEdge env? Can you give me some cmds to run.

What machine are you using? If is a customer one, please post to #6921.

3. Can you confirm outputToFile logs output as expected?

Yes, in FWD it works as expected for appserver agents, I've tested with 6129b.

How are you testing the appserver in FWD? Are you just starting the appservers? Keep in mind that outputToFile can be configured for either batch or agent FWD clients, so if you can test batch programs and it works there, then is OK.

Please test this in a batch program, started both via command line and ./server.sh -b or directory.xml scheduler:

message "this is error1".  // logged in both
message "this is error2" view-as alert-box. // logged in both

display "display is emitted only in batch". // this is logged in batch, but not in appserver agent
put screen row 1 col 1 "put is not emitted". // this is not logged in either batch or appserver agent

// in appserver agent, there is always a default log, and this is written to that log
// for batch, if no log is opened, 'Cannot write message to log, as there is no log open (14332)' is shown on screen
log-manager:write-message("this is logged").

def var v as handle.
do on error undo, leave:
   delete object v. // logged in both
end.

So configuration-wise appserver turns into batch if I remove <node-attribute name="appserver" value="app_server"/> from directory.xml:

<node class="container" name="">
  <node class="container" name="security">
    <node class="container" name="accounts">
      <node class="container" name="processes">
        <node class="process" name="appserver_process">
          <node-attribute name="appserver" value="app_server"/>

If the same procedure is run as appserver, it doesn't output anything. So is this the expected behavior? In that case is it expected that only rest / soap agents write to outputToFile, when endpoints are hit? How is outputToFile supposed to be used with appservers?

Galya, please describe how you are running a program on the appserver. A batch program executes its 4GL program immediately. In the appserver case, you need another 4GL program to connect to the appserver and run the 4GL program.

Constantin Asofiei wrote:

Galya, please describe how you are running a program on the appserver. A batch program executes its 4GL program immediately. In the appserver case, you need another 4GL program to connect to the appserver and run the 4GL program.

I've used the new testcases template as it is, which supports appserver scheduled for immediate run. The procedure though didn't set an appserver and no agents were connected. My question is if we expect the appserver procedure (the server-side) to produce output to outputToFile the same way as a scheduled batch? Since StandardServer always returns false (for running) to the client JVM right away, it will never do.

P.S. I'll try to test the original behavior in OpenEdge soon, but if we already have it described somewhere it would be helpful to know.

Code Review Task Branch 5701a Revisions 14484 and 14485

1. In spawn.c:launchP2JClient, the memset(arguments[length + 1], '\0', len); should use len * sizeof(char) instead of len. The winspawn.c has the same issue.

2. Please add the history number to your history entries. For example, use 031 for ClientCore, 005 for ProcessClientBuilder and so forth.

3. Please add javadoc for ClientCore.setOutputToFile().

Eugenie: Please review branch 5701a.

Greg Shah wrote:

Eugenie: Please review branch 5701a.

I have no objections, in addition to history entries it is required to change Year to 2023.

Also in winspawn.c I would add the initial setup for module static var szOutputFile[MAX_SIZE] = "". But it is just a style preferences I guess, the current version should also work.

Greg Shah wrote:

Code Review Task Branch 5701a Revisions 14484 and 14485

1. In spawn.c:launchP2JClient, the memset(arguments[length + 1], '\0', len); should use len * sizeof(char) instead of len. The winspawn.c has the same issue.

https://en.cppreference.com/w/cpp/language/sizeof says:

The following sizeof expressions always evaluate to 1:

sizeof(char)
sizeof(signed char)
sizeof(unsigned char)

I found the same statement in other sources as well.

Maybe what we're looking for according to Stackoverflow is sizeof(char*)?

char is a character and sizeof(char) is defined to be 1. (N1570 6.5.3.4 The sizeof and _Alignof operators, paragraph 4)

char* is a pointer to a character and sizeof(char*) depends on the environment. It is typically 4 in 32-bit environment and 8 in 64-bit environment.

Also my tests show that the string is not cut, but on other env it might not work.

What about changing it the linux version from:

1021      int len = strlen(output_file_name) + 1;
1022      arguments[length + 1] = (char*) malloc(len * sizeof(char));
1023      memset(arguments[length + 1], '\0', len);
1024      strcpy(arguments[length + 1], output_file_name);

to:

1021      int size = (strlen(output_file_name) + 1) * sizeof(char*);
1022      arguments[length + 1] = (char*) malloc(len * sizeof(char*));
1023      memset(arguments[length + 1], '\0', size);
1024      strcpy(arguments[length + 1], output_file_name);

Please note the change on line 1022. It should apply to line 1059 as well. The changes will be applied to win as well.

About the other points, I'm on it.

Eugenie Lyzenko wrote:

I have no objections, in addition to history entries it is required to change Year to 2023.

It's 2023 for the entries related to 5701, but there were no history entry numbers and maybe you saw the ones for the previous task. When I add the numbers it will look fine.

Also in winspawn.c I would add the initial setup for module static var szOutputFile[MAX_SIZE] = "". But it is just a style preferences I guess, the current version should also work.

It feels weird to a Java developer to initialize an array that way and there is a mixture of both styles in the file, but I'll make the change for this one.

Galya Bogdanova wrote:

Eugenie Lyzenko wrote:

I have no objections, in addition to history entries it is required to change Year to 2023.

It's 2023 for the entries related to 5701, but there were no history entry numbers and maybe you saw the ones for the previous task. When I add the numbers it will look fine.

I meant the following:

** Copyright (c) 2013-2021, Golden Code Development Corporation.
...
**     EVL 20210519 Fix for segmentation fault in strcmp() library call.  Added new error codes to inform
**                  the caller about system errors.

When you add new history entry with date (2023MMDD), and you should mark your change the year on the top of the header should match new entry:

** Copyright (c) 2013-2023, Golden Code Development Corporation.
...
** 014 GBB 20230201 Some text.

We need to avoid any modifications without history log. It will be very difficult to understand what and when was changed otherwise.

Eugenie Lyzenko wrote:

When you add new history entry with date (2023MMDD), and you should mark your change the year on the top of the header should match new entry:

Did you review file 5701a-r14485.diff ? There are 4 source files modified and all of them have updated year.

Galya Bogdanova wrote:

Eugenie Lyzenko wrote:

When you add new history entry with date (2023MMDD), and you should mark your change the year on the top of the header should match new entry:

Did you review file 5701a-r14485.diff ? There are 4 source files modified and all of them have updated year.

Yes, I did. src/native/spawn.c has neither history nor year update. src/native/winspawn.c has last update entry as of GBB 20221209.

Galya Bogdanova wrote:

Eugenie Lyzenko wrote:

Yes, I did. src/native/spawn.c has neither history nor year update. src/native/winspawn.c has last update entry as of GBB 20221209.

There are 4 .diff files attached to the task, the latest is called 5701a-r14485.diff and below is its content.

I downloaded it from the task 5 times or more and it's the same every time. Is it what you're seeing?

OK. Now I've got the versions you mentioned.

It is 14486, not 14485, right? So far I have no notes for 14486. I think this is versions misunderstanding...

Lets consider this code from spawn.c:

   char **arguments = (char**) malloc((length + 2 + output_to_file_opt_found) * sizeof(char*));
   arguments[0] = NULL;
   arguments[length + 1 + output_to_file_opt_found] = NULL;

   if (output_to_file_opt_found == 1)
   {
      int len = strlen(output_file_name) + 1;
      arguments[length + 1] = (char*) malloc(len * sizeof(char));
      memset(arguments[length + 1], '\0', len);
      strcpy(arguments[length + 1], output_file_name);
   }

arguments is a char** which means "pointer to a char pointer". This is a technique in C (and C++) in which one allocates a region of memory for an array of pointers. The pointers can be accessed via array syntax (arguments[0] or arguments[length - 1]) or via direct pointer arithmetic.

Each pointer in this array, is a pointer to char data. That pointer references a separate block of memory that needs to be separately allocated (via malloc() or other means).

The reference to arguments[length + 1] in memset(arguments[length + 1], '\0', len); is passing the array element at length + 1 which is a char* (a pointer to char data). memset() is going to blast a bit pattern into the memory referenced by that block. That referenced memory was just allocated using arguments[length + 1] = (char*) malloc(len * sizeof(char));. The size of that block is len * sizeof(char).

When you use memset() to then overwrite that block with the '\0' char, you must do that with the proper size which is sizeof(char), the same thing used in the malloc().

I highly recommend the bible of C programming by Kernighan and Ritchie: The C Programming Language (2nd Edition). This is a great (and small) book that does an excellent job of teaching the language. To quote the authors (who also invented the language):

"C is not a big language, and it is not well served by a big book."

Chapter 5 is about pointers and arrays. It explains this all quite well. Feel free to take some time to read the book.

The following sizeof expressions always evaluate to 1:

Yes, that is generally true, but it is not guaranteed in the language spec so it is a good practice to use sizeof(char) to be safe.

Maybe what we're looking for according to Stackoverflow is sizeof(char*)?

No, this would would be a buffer overflow for sure. char* is a pointer, which on our systems is 64-bits in size.

If sizeof(char) is 1 (8 bits!) using sizeof(char* in memset() will overwrite memory outside of our valid allocated area. Because malloc() (from the C runtime or "CRT") was used, the memory allocated came from the C language "heap" which is a large block of memory that gets suballocated as requested (e.g. by malloc()). This means it is not guaranteed to overflow the allocated operating system memory region (which would cause an abend like a trap E/protection fault/segmentation fault on Intel x86_64/AMD64 architecture) but it might. We don't know what exactly would happen but we do know that we would overwrite (corrupt!) a bunch of memory that we don't want to touch.

Code Review Task Branch 5701a Revisions 14485 and 14486

The changes are good.

At this point, we need to pause because of the freeze on trunk. We will rebase and merge when the "thaw" occurs.

Greg, I merged the changes to an up-to-date branch 5701b and there were no conflicts (r14485). When do I merge?

It won't be until some time next week. We have many performance changes and some regression fixes that need to go first.

Please rebase 5701b from trunk rev 14505. Then if it passes your basic tests, you can merge to trunk.

5701b is merged to trunk under r14506 and works fine, but there are regression issues with the previous revision r14505. Who was responsible for testing r14505?
Fix for the broken redirect condition in r14507.

Galya, there is a regression in trunk revs 14506/14507.

This can be seen with appservers, where there is a MESSAGE "Message" VIEW-AS ALERT-BOX INFO BUTTONS OK. statement in an appserver call.

Can you point me where this issue may be?

Constantin Asofiei wrote:

This can be seen with appservers, where there is a MESSAGE "Message" VIEW-AS ALERT-BOX INFO BUTTONS OK. statement in an appserver call.

What is the unexpected behavior? The only change in #5701 is redirecting System.out for appserver / scheduled batch processes.

Galya Bogdanova wrote:

Constantin Asofiei wrote:

This can be seen with appservers, where there is a MESSAGE "Message" VIEW-AS ALERT-BOX INFO BUTTONS OK. statement in an appserver call.

What is the unexpected behavior? The only change in #5701 is redirecting System.out for appserver / scheduled batch processes.

• in ProcessClientBuilder.getSpawnArguments there is this code:
  

        if (outputToFile != null)
        {
           cmd.add("-O");
           cmd.add("server:clientConfig:outputToFile=" + outputToFile);
        }
  

• later on, in spawn.c around line 1300 there is this code:
  

        for (i = MIN_ARGS_NO_PASSWORD; i < argc; i++)
        {
           /* -O <outputToFile> */
           if ((strcmp("-O", argv[i]) == 0) && (++i < argc))
           {
              strcpy(output_file_name, argv[i]);
              output_to_file_opt_found = 1;
           }
        }
  

  
  Here, output_file_name gets assigned to server:clientConfig:outputToFile=somefile.log. There is no check to remove the '=' from the argument. I'm not sure if this is expected or not.
• this, in turn, uses it in launchP2JClient, this code:
  

     if (output_to_file_opt_found == 1)
     {
        int size = (strlen(output_file_name) + 1) * sizeof(char);
        arguments[length + 1] = (char*) malloc(size);
        memset(arguments[length + 1], '\0', size);
        strcpy(arguments[length + 1], output_file_name);
     }
  

  
  which just passes the 'bootstrap config and filename' to the arguments for the JVM launch. I think this is OK.

But, in filesys.c, there is this code:

JNIEXPORT jboolean JNICALL
   Java_com_goldencode_p2j_util_FileChecker_isStdoutRedirected(JNIEnv *env, jclass  jc)
{
   struct stat st;

   return (fstat(STDOUT_FILENO, &st) == 0) && // Means success call to fstat
          (S_ISCHR(st.st_mode) == 0) &&       // The output is not character device
          (S_ISREG(st.st_mode) != 0);         // but regular file
}

This patch seems to solve the issue:

--- old/src/com/goldencode/p2j/util/FileChecker.java
+++ new/src/com/goldencode/p2j/util/FileChecker.java
@@ -75,6 +75,7 @@

 import java.io.File;

+import com.goldencode.p2j.ui.client.chui.driver.batch.BatchHelper;
 import com.goldencode.util.*;

 /**
@@ -291,6 +292,6 @@
          consoleRedirected = Boolean.valueOf(isStdoutRedirected());
       }

-      return consoleRedirected.booleanValue();
+      return consoleRedirected.booleanValue() || BatchHelper.IS_OUTPUT_REDIRECTED.get();
    }
 }

But I don't know if this is the correct fix.

Constantin, this is troublesome. With #5701 stdout is not redirected in the native code, but programmatically in Java and FileChecker#isConsoleRedirected does not indicate that. I actually wasn't aware of that native method while depeloping, so I'll have to make amends now.

Constantin Asofiei wrote:

This patch seems to solve the issue:
[...]

But I don't know if this is the correct fix.

Yes, this is what should be done.

Even better if IS_OUTPUT_REDIRECTED is moved to FileChecker.

The 4GL knows when its STDIO is redirected and it changes its behavior based on that. I think we implemented the Java_com_goldencode_p2j_util_FileChecker_isStdoutRedirected() to duplicate that behavior.

Eugenie: Am I remembering correctly?

If so, we should not remove it.

Greg Shah wrote:

The 4GL knows when its STDIO is redirected and it changes its behavior based on that. I think we implemented the Java_com_goldencode_p2j_util_FileChecker_isStdoutRedirected() to duplicate that behavior.

Eugenie: Am I remembering correctly?

Yes.

If so, we should not remove it.

Yes, we need this call to be in native code.

Greg, I think you are right. This may affect the redirected terminal in both ChUI and batch processes.

For batch processes, IIRC, STDOUT is the default destination for all UI statements. And OUTPUT TO can redirect it to a file (and this applies to ChUI/GUI, too).

There is at least this code in BatchPrimitives which I think needs to now real STDOUT redirection, and not the fact that 'output has been redirected to a file (or not)':

   @Override
   public void updateScreen(ScreenData screen)
   {
      if (isActive)
      {
         // Special processing for redirected STDOUT and Windows background mode
         if (FileChecker.isConsoleRedirected() ||
             (PlatformHelper.isUnderWindowsFamily() && OutputManager.isBatchInBackground()))
         {

We may need to choose carefully where the IS_OUTPUT_REDIRECTED flag is used, not just add it to FileChecker.isConsoleRedirected().

Galya: please do some tests in batch/appserver/ChUI, with code like this:

OUTPUT TO somefile.txt.
message "something".
output close.

This is the kind of STDOUT redirection I refer above.

I didn't receive notification for the notes from my last comment, so let me get up to date...

Updated by Galya Bogdanova about 1 hour ago

    billable changed from No to Yes
    Assignee deleted (Galya Bogdanova)
    Priority changed from Normal to Low
    % Done changed from 80 to 0

Lol. My... Is this the overwrite message warning for... I haven't set this myself.

Galya Bogdanova wrote:

Lol. My... Is this the overwrite message warning for... I haven't set this myself.

If someone else changes the status/priority/assignee at the task, and another person submits something without refreshing the page, Redmine gets 'confused' and messes up these.

Constantin Asofiei wrote:

Galya: please do some tests in batch/appserver/ChUI, with code like this:
[...]

If I understand right, to test the appserver I need to run a batch client for example that connects to appserver and runs the output to code on it.

I thought I knew how to do this, but any connect args I try, don't work with the new testcases, for example:

create server h.
h:connect("-H localhost -S 3333 -DirectConnect -AppService app_server").

h:connect("-AppService app_server -sessionModel Session-free").

h:connect("-H localhost -S fwd1 -sessionModel Session-free").

It's easy with the NS in 4GL. Here I get lost how to connect to the appserver. Maybe I'm missing configs.

Otherwise chui (terminal / swing) and batch work fine - generate the file and output any other display text.

Galya, try this for appserver connect -AppService app_server -S 21200 -H localhost -sessionModel Session-free -DirectConnect.

Did you test the batch client with a program using OUTPUT TO somefile.txt?

Constantin Asofiei wrote:

Did you test the batch client with a program using OUTPUT TO somefile.txt?

Yes. I've just tested appserver as well. The procedure I was running is doing output to, output stream to, showing alert-box and throwing AppError. The 2 files are created with all drivers. The two messages are recorded in outputToFile (for appserver), or to > (for batch), or displayed in chui terminal and swing apps.

using Progress.Lang.AppError.
using Progress.Lang.Object.

define variable oErr as AppError no-undo.
define stream stostream.

output stream stostream to value( "output.stream" ).

put stream stostream unformatted
"hello" 
skip.

output stream stostream close.

output to output.to.
message "hello".
output close.

message "ERROR" view-as alert-box error.

oErr = new AppError('Error message from AppError constructor', 1) no-error.
oErr:AddMessage('Error message from AppError AddMessage', 2) no-error.
undo, throw oErr.

I've found a weird behavior just now. If the output to file is already existing, the text goes to outputToFile in appserver run. Let me verify if that was the original behavior.

Ok, that's straight weird. Every third run of the same procedure (without deleting the output to files) writes the file texts to outputToFile. I'm testing on the very old r14483. So the conclusion is this odd behavior is not related to #5701.

[?1049h(B[?7h[?1h=ERROR
Error message from AppError constructor
ERROR
Error message from AppError constructor
hello
hello
ERROR
Error message from AppError constructor
ERROR
Error message from AppError constructor
hello
hello
ERROR
Error message from AppError constructor
ERROR
Error message from AppError constructor
hello
hello
ERROR
Error message from AppError constructor

Constantin, I think it is good to go... for 3rd time. 3rd time should be the lucky one.

Galya, the changes in 5701d look OK.

• runappsrv.p:
  

  def var hs as handle.
  
  create server hs.
  message hs:connect("-AppService catest -sessionModel Session-free").
  
  run msgtest.p on server hs (input "this is a test0").
  run msgtest2.p on server hs (input "this is a test1").
  run msgtest2.p on server hs (input "this is a test2").
  run msgtest2.p on server hs (input "this is a test3").
  message hs:disconnect().
  

• msgtest.p:
  

  def input param ch as char.
  
  MESSAGE "Message:" ch VIEW-AS ALERT-BOX INFO BUTTONS OK.
  

• msgtest2.p:
  

  using Progress.Lang.AppError.
  using Progress.Lang.Object.
  
  def input param ch as char.
  
  define variable oErr as AppError no-undo.
  define stream stostream.
  
  output stream stostream to value( "b.txt" ) append.
  
  put stream stostream unformatted
  "foo" ch
  skip.
  
  output stream stostream close.
  
  output to a.txt append.
  message "bar" ch.
  output close.
  
  message "ERROR" ch view-as alert-box error.
  
  oErr = new AppError('Error message from AppError constructor ' + ch, 1) no-error.
  oErr:AddMessage('Error message from AppError AddMessage ' + ch, 2) no-error.
  undo, throw oErr.
  

which shows these logs:

[23/03/30@09:12:25.849+0300] P-004640 T-004644 1 AS -- (Procedure: 'msgtest.p' Line:3) Message: this is a test0
[23/03/30@09:12:25.857+0300] P-004588 T-004592 1 AS -- (Procedure: 'msgtest2.p' Line:18) bar this is a test1
[23/03/30@09:12:25.858+0300] P-004588 T-004592 1 AS -- (Procedure: 'msgtest2.p' Line:21) ERROR this is a test1
[23/03/30@09:12:25.858+0300] P-004588 T-004592 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError constructor this is a test1
[23/03/30@09:12:25.858+0300] P-004588 T-004592 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError AddMessage this is a test1
[23/03/30@09:12:25.859+0300] P-004608 T-004612 1 AS -- (Procedure: 'msgtest2.p' Line:18) bar this is a test2
[23/03/30@09:12:25.859+0300] P-004608 T-004612 1 AS -- (Procedure: 'msgtest2.p' Line:21) ERROR this is a test2
[23/03/30@09:12:25.859+0300] P-004608 T-004612 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError constructor this is a test2
[23/03/30@09:12:25.859+0300] P-004608 T-004612 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError AddMessage this is a test2
[23/03/30@09:12:25.861+0300] P-004640 T-004644 1 AS -- (Procedure: 'msgtest2.p' Line:18) bar this is a test3
[23/03/30@09:12:25.861+0300] P-004640 T-004644 1 AS -- (Procedure: 'msgtest2.p' Line:21) ERROR this is a test3
[23/03/30@09:12:25.861+0300] P-004640 T-004644 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError constructor this is a test3
[23/03/30@09:12:25.861+0300] P-004640 T-004644 1 AS -- (Procedure: 'msgtest2.p' Line:25) Error message from AppError AddMessage this is a test3

Message:this is a test0
ERRORthis is a test1
Error message from AppError constructor this is a test1
Error message from AppError constructor this is a test1
Error message from AppError AddMessage this is a test1
ERRORthis is a test2
Error message from AppError constructor this is a test2
Error message from AppError constructor this is a test2
Error message from AppError AddMessage this is a test2
bar this is a test3
bar this is a test3
ERRORthis is a test3
Error message from AppError constructor this is a test3
Error message from AppError constructor this is a test3
Error message from AppError AddMessage this is a test3

Keep in mind this is with the 5701d fix and FWD trunk. So FWD trunk loses some MESSAGE statements when writing them to log. Can these already be fixed by your other log-related tasks? Otherwise, we need to open a bug for this.

Constantin Asofiei wrote:

Keep in mind this is with the 5701d fix and FWD trunk. So FWD trunk loses some MESSAGE statements when writing them to log. Can these already be fixed by your other log-related tasks? Otherwise, we need to open a bug for this.

I would say let's open the bug and block it until all log related tasks are merged, because it's confusing for me as well at this stage. Let's test it again after it's all done.

I think I did something to the repeated constructor msg, but atm not 100% sure.

I added some missing messages to the clientlog for #5753.

Galya Bogdanova wrote:

Constantin Asofiei wrote:

Keep in mind this is with the 5701d fix and FWD trunk. So FWD trunk loses some MESSAGE statements when writing them to log. Can these already be fixed by your other log-related tasks? Otherwise, we need to open a bug for this.

I would say let's open the bug and block it until all log related tasks are merged, because it's confusing for me as well at this stage. Let's test it again after it's all done.

I agree, create a bug in Base Language and link it as related task to #5701 and other logging tasks.

I was able to reproduce the issue with the hanging appserver and it's fixed in 5701d as I've mentioned earlier. Do I merge?

Galya Bogdanova wrote:

I was able to reproduce the issue with the hanging appserver and it's fixed in 5701d as I've mentioned earlier. Do I merge?

Yes, rebase 5701d from trunk rev 14520 and merge it.

Merged to trunk as r14521. It also fixes the repeating of log messages mentioned in #5753#note-79.

Do I have to write an email for the fix?

Galya Bogdanova wrote:

Merged to trunk as r14521. It also fixes the repeating of log messages mentioned in #5753#note-79.

Do I have to write an email for the fix?

Yes, when a branch is merged to trunk, an email notification must be sent to the entire team. Also, make sure to archive the branch.