FWD » Core Development » Base Language

See https://projsrv01/redmine/issues/1611#note-36 for some useful notes.

Please provide a summary of your findings so far.

I know that the DOS statement needs conversion support. I would like to know if there are any other conversion changes needed. I would like you to propose the conversion changes and AFTER we have discussed them, you will implement the changes. Then you will temporarily put this task on hold to work on other tasks.

At this time I have not found any other statements except DOS to add the conversion support. As far as I understood the statements OS-COMMAND, I/O THROUGH, UNIX and OS-COMMAND that currently converted are using the common ProcessOps.launch() wrapper to pass the control to native code.

And for now I see two possible ways to implement Windows specific support:

1. We detect the Windows inside the Java code just before calling process related command and then execute Windows alternative for ProcessOps.launch(), something like ProcessOps.launchWin(). This new wrapper is implementing in Windows version of the process.c file and will be located in p2j.dll.

2. We do not make any separation on the conversion level. Just add the support for DOS statement by adding(or extending another words) dos commands to the the UNIX commands subset. As the result - we will have the common set of the OS Shell commands including DOS, UNIX, OS-COMMANDS... calling to ProcessOps.launch(). If the OS is matches the requested command - it will be executed in native code, if not - we will have error/exception generating error info to P2J.

From the options above I'm inclining to choose #2. I think(correct me if I'm wrong) the native Progress 4GL does not check this correspondence on the compilation level. The responsibility to choose the right command for appropriate OS is up to Progress programmer. This has the dark side of course - conversion error propagation up to the runtime code when for example we can try to execute Windows command under Linux and vise versa.

I have installed the MinGW development toolkit on my Windows system but did not experiment with it long enough. The point I need to investigate: if the API of this toolkit is fully compatible with GNU C we use to compile the native code under Linux. If the answer is positive - we can use the similar process/pipe calls in both code version(Linux and Windows) with possible modifications to reflect Windows specific behavior of the native 4GL code. Probably this means we will have to distribute the C runtime(that translates GNU C calls to Win API ones) from MinGW to the customers.

Option 2 is certainly how we want to do it. Yes, it is up to the application to only try to run child processes that can be run.

The process launching code in ProcessDaemon will need modification since it has some Linux/UNIX specific stuff (like a hard coded "sh").

 The point I need to investigate: if the API of this toolkit is fully compatible with GNU C we use to compile the native code under Linux. If the answer is positive - we can use the similar process/pipe calls in both code version(Linux and Windows) with possible modifications to reflect Windows specific behavior of the native 4GL code. Probably this means we will have to distribute the C runtime(that translates GNU C calls to Win API ones) from MinGW to the customers.

It is my understanding that mingw does NOT HAVE its own compatible GNU runtime environment. In fact, that is exactly why we are picking it. We know that there are many native features of Windows that cannot be supported or duplicated using the GNU runtime. Instead of trying to start down a path that can never get us to the proper destination, I want to implement using the WIN32 APIs directly.

For now, just make sure that the DOS command is fully supported in conversion. Put everything else (the runtime changes) on hold.

Looks like we now already have properly conversion for DOS statement. The following line
dos dir /w > dir.log.
converts now to:
ProcessOps.launch(new String[] {
"dir",
"/w",
">",
"dir.log"
}, false, true);
And the code can be compiled.

So any arbitrary DOS a b c ... x y z. command will be converted to launch( new String[] { "a", "b", "c", ... , "x", "y", "z" } ). Is this the support we are looking for on conversion level?

As to native support I would suggest to separate the platform specific code to separate file(or even two). Then each JNI:
Java_com_goldencode_p2j_util_LaunchManager_someFunction()
#if TARGET_OS == WINDOWS
someFunctionWin()
#else
someFunctionLinux()
#endif

This allows us to leave the current working Linux code untouched and concentrate on Windows implementation instead of mixing two platform code within the same function.

And looks like at the moment of native implementation we will have to modify ProcessDaemon.prepareCommandLine() method replacing "sh" with "cmd.exe" for Windows case.

So any arbitrary DOS a b c ... x y z. command will be converted to launch( new String[] { "a", "b", "c", ... , "x", "y", "z" } ). Is this the support we are looking for on conversion level?

Yes, you are right. We already have the proper support in the conversion for the DOS command (and also the undocumented OS2, BTOS and VMS <g>). The rules are in rules/convert/process_launch.rules.

You don't need to do anything on the conversion.

As to native support I would suggest to separate the platform specific code to separate file(or even two). Then each JNI:
Java_com_goldencode_p2j_util_LaunchManager_someFunction()
#if TARGET_OS == WINDOWS
someFunctionWin()
#else
someFunctionLinux()
#endif

OK.

And looks like at the moment of native implementation we will have to modify ProcessDaemon.prepareCommandLine() method replacing "sh" with "cmd.exe" for Windows case.

Yes. And I would be surprised if we didn't find some other things that need some modification. But for now, you can put this on hold and work on other conversion tasks for milestone 4.

The question. I have idea to install The Windows inside the KVM Virtual Machine on my new Ubuntu box. Will it also conflict(as having dual boot system) with the current security requirements for the customer's development system?

Yes, I have another standalone system with Windows on the board for such experiments but this is a bit less convenient.

You are allowed to use Windows in KVM on the same Ubuntu system. It will be allowed in the standards. Thanks for asking.

Feedback:

It is OK so far. Make sure you merge with the latest version of P2J (for example the build.xml that you started from is out of date). Also, I want you to integrate your changes into the main build.xml (we don't want a separate build.xml for win32), but it is fine to make a separate copy for experiments. The final result must automatically detect the platform on which the build is occurring and build the right native module (.so or .dll) for that platform.

In regard to the structure of the windows vs linux code, I want to do the following:

1. Each JNI function can have as much common code as is possible to be used on both platforms.

2. Any code that needs to be different for each platform should be in a separate .c file that is platform specific (one for each platform). For example, you would have a file_system_helper_win.c and a file_system_helper_linux.c and only one would be compiled and linked into the result, depending on the platform. If you need to differentiate signatures (e.g. for calling conventions or something like that), you can have separate header files too.

3. The names of the platform specific functions should not have the platform names in them. Instead, the platform-specific functions will have the same names. For example, both file_system_helper_win.c and file_system_helper_linux.c would have a checkFileType() function with the same signature.

4. Using this approach, the JNI functions will call the helper function without needing to IFDEF.

JNIEXPORT jint JNICALL 
       Java_com_goldencode_p2j_util_FileChecker_checkFileType(JNIEnv  *env,
                                                              jclass  jc, 
                                                              jstring fname) 
{
   checkFileType(env, jc, fname);
}

This should greatly reduce the amount of conditional preprocessing that has to occur. But it means that you must design the platform-specific helper API to cleanly separate the common code from the platform-specific code. Of course, please maximize the common code.

I am generally fine with the build.xml approach. The only thing I see is this:

    <javah class="com.goldencode.p2j.util.LaunchManager" 
           destdir="${src.home}/native" 
           classpath="${build.home}/classes"/>
      <javah class="com.goldencode.p2j.util.FileChecker" 
           destdir="${src.home}/native" 
           classpath="${build.home}/classes"/>
    <copy todir="${build.home}/native">
      <fileset dir="${src.home}/native" includes="makefile" />
    </copy>

This code is common and should be in a target that is shared by all the other native tasks as a "depends".

In regard to the problems detecting win 32-bit vs 64-bit, go with your approach for now. Please put some comments into the build.xml to explain why you have made the choices you did.

I like it. Keep going in this direction.

Code Feedback:

1. Please remove the following from future updates:

src/native/com_goldencode_p2j_util_FileChecker.h  
src/native/com_goldencode_p2j_util_LaunchManager.h

Those are generated files and should not be distributed.

2. Please use the "simple" process.h and filesys.h instead of the longer *_helper.h versions.

3. Please use a simpler process_linux.c/process_win.c instead of the helper.c versions. Do the same thing with the file_system_*.c files.

4. This code is missing from process_helper_linux.c:

   // due to a bug in the JVM 1.4.2 and earlier, we must override any
   // signal handler for SIGCHLD that may exist because otherwise the JVM
   // may reap our process instead of letting us do it (see Sun J2SE bugs 
   // #4945203, #4785154 and #4763362)
   signal(SIGCHLD, SIG_IGN);

I think it is pretty important and should be put back in, unless you have a good reason that it needs to be removed.

Other than the above 4 items, I like the way this is shaping up. Keep going.

2. Please use the "simple" process.h and filesys.h instead of the longer *_helper.h versions.

I had the same idea. But I did worry process.h can conflict with mingw internal include file with the same name - process.h. I made just name separation.

3. Please use a simpler process_linux.c/process_win.c instead of the helper.c versions. Do the same thing with the file_system_*.c files.

OK.

4. This code is missing from process_helper_linux.c:

> // due to a bug in the JVM 1.4.2 and earlier, we must override any
> // signal handler for SIGCHLD that may exist because otherwise the JVM
> // may reap our process instead of letting us do it (see Sun J2SE bugs 
> // #4945203, #4785154 and #4763362)
> signal(SIGCHLD, SIG_IGN);
> 

I think it is pretty important and should be put back in, unless you have a good reason that it needs to be removed.

OK. Sorry I've just lost this code during migration to the platform independent coed and back. Will be restored.

Reworked file names to simplify. Unfortunately for now process.h has compilation issue in Windows. So the file is: process_.h

Let's call it process_launch.h so that it is descriptive but still won't conflict.

Overall, the latest version is really looking good. Keep going.

4. The process launching has some degree of working.

That is great!!! Well done!

What do you think about C code GUI debugger for Windows we can use in this project? It will be good to have something other than gdb provided with mingw.

Look at this:

http://www.gnu.org/software/ddd/
http://www.drdobbs.com/tools/visual-debugging-with-ddd/184404519 (at the end it describes that using Cygwin, one can get DDD running)

I have used it on Linux as my primary C/C++ debugger. I haven't used it much, but it has done the job. It uses gdb as its backend, so it should work for debugging mingw created binaries too. But there will be a bit of extra work in getting it running under Windows.

The issues(related or partially) to discuss so far:

1. The source file com/goldencode/p2j/util/SourceNameMapper.java method initMappingData, line 832:
...
sb.append(pkgroot.replaceAll("\\.", fileSep));
...
This call generates exception in Windows(for fileSep == "\"). This is known issue we fixed some time ago.
I would suggest to replace this call with:
...
sb.append(pkgroot.replaceAll("\\.", StringHelper.fixupRegex(fileSep)));
...
This will add second "\" for Windows and do nothing for Linux.

2. The source file com/goldencode/p2j/util/ProcessDaemon.java method launch, line 156:
...
terminal.suspend();
...
Do we really need this call here? The result of this call is invoking TypeAhead.suspend which calls Object.wait() and suspend the thread execution before launch the command. And it is required to press spacebar to continue. This is not happening neither in Windows 4GL nor in Linux 4GL(in both silent and non-silent modes). To properly implement 4GL behaviour we need to remove this(fully or partially by substitution with another thin client clearing code).

I would suggest to replace this call with:
...
sb.append(pkgroot.replaceAll("\\.", StringHelper.fixupRegex(fileSep)));

OK, include it.

terminal.suspend();
...
Do we really need this call here? The result of this call is invoking TypeAhead.suspend which calls Object.wait() and suspend the thread execution before launch the command.

Yes, this code is essential. It does the following:

1. It stops the TypeAhead class from trying to read keystrokes. If it did not do this, then we would be "fighting" with the child process for input.

2. It notifies the Window class that it is suspended. This disables drawing to the window (except for the status line).

3. It clears the terminal to remove all the previous 4GL output.

4. It suspends processing at the NCURSES level.

All of these things are essential. Of course, we have no such terminal facilities written yet for Windows, so in that case we may have to bypass this stuff. But it MUST be there for Linux.

And it is required to press spacebar to continue. This is not happening neither in Windows 4GL nor in Linux 4GL(in both silent and non-silent modes).

This comes from the following code (line 194 of ProcessDaemon):

         // if not silent we display a default message and pause indefinitely
         if (!silent)
         {
            lastkey = terminal.suspendedPause();
         }         

This is only done in non-silent mode. But I am sure it is needed. You can see this in a very simple testcase. Run this on lindev01 (Linux 4GL environment):

os-command bash.

Do something at the command line (e.g. ls -l). Then end the child process by typing "exit" and hitting ENTER. You will see the "Press space bar to continue." message and the indefinite pause.

It also happens in something like this:

os-command ls -l.

We know for sure this is working today. Perhaps there is a difference in Windows, but for Linux it is required.

To properly implement 4GL behaviour we need to remove this(fully or partially by substitution with another thin client clearing code).

It should still work in the Swing CHUI client, right?

If there are specific differences in Windows, please detail how they are different from the Linux 4GL environment. Show 4GL example code and the differing results so I can understand.

It looks good. Keep going.

Good work! Keep going.

Update status for INPUT-OUTPUT THROUGH. The idea of this 4GL statement is to use external program instead of console to display data, modify and commit the result back. Consider the simple test example:

define variable intVar as integer initial 4.

display intVar.
message "Press any key to change the value".
pause.

input-output through chvalue.exe unbuffered.
export intVar.
set intVar.
input-output close.

display intVar.
message "The value should be changed by external program".
pause.

The chvalue.c program used here is:

#include <stdio.h>

main()
{
   int value;

   setbuf(stdout, (char *) NULL);

   while (scanf("%d", &value) == 1)
   {
      value = value * 2 + 5;
      printf("%d\n", value);
   }
}

Converted and run on Windows 32-bit and 64-bit. So the INPUT-OUTPUT THROUGH is working with the recent update. The only question here is: We have the Waited thread in ../p2j/util/ProcessStream.java. It is OK when we have either only INPUT THROUGH or OUTPUT THROUGH redirection - we have only one reader or writer to wait for. But in the case of INPUT-OUTPUT THROUGH we have both writer and reader and as results two Waiter threads for single external process. I just afraid of concurrency for these two wait calls in different threads. Not sure if this is a problem for native code calls.

And one more conclusion: MinGW is OK for this native implementation. Continue working.

The code continues to look good. Your approach makes sense. Good stuff. Keep going.

Please review this:

http://support.microsoft.com/kb/94248

Please review this:

http://support.microsoft.com/kb/94248

The question. I have checked out P2J.DLL. Both 32-bit and 64-bit versions are linked to be used with MSVCRT.DLL and KERNEL32.DLL in MinGW if we do not specify explicitly the entry point. These libraries are the part of the Operating System so we do not need to distribute them separately.

Do we need to implement out own DLL Init routine? Or link to the special version of CRT library(for example LIBC.LIB or LIBCMT.LIB)? We consider the P2J.DLL to be multithreaded?

Do we need to implement out own DLL Init routine?

Yes, I think we should. For now, we just need to properly handle the CRT initialization/termination on process attach/detach. This is pretty much the same as we used to do back in OS/2.

We consider the P2J.DLL to be multithreaded?

Yes, everything in the DLL must be thread-safe and re-entrant.

It is true that the 4GL is single threaded, but I don't want to assume that the DLL will never be accessed by more than 1 thread at a time. So we must make it safe.

It looks good. Keep going.

Please make a list of everything that remains to be done.

Please make a list of everything that remains to be done.

At this moment I see the following TODO list(for process start related task):

1. Write the DLL Initialization routine and adjust the native code building makefile to link with appropriate Windows LIB/DLL.
2. Investigate the signal handling behaviour, particularly the ability to replace sigemptyset() function and handling the following signals: SIGTSTP, SIGTTIN, SIGTTOU.
3. Investigate the requirement to add replacement for Linux fix with signal(SIGCHLD, SIG_IGN); in Windows code.
4. Write the set of the process-related samples(we certainly need more test than we now have) to verify conversion-runtime cycle.

The question. I'm planning to separate DLL Windows initialization code in the file entry_win.c. Is this name acceptable?

I'm planning to separate DLL Windows initialization code in the file entry_win.c. Is this name acceptable?

Please use initterm_win.c instead.

Experiments with different testcases show the following results for process launch 4GL options for Windows implementation of the P2J native library and runtime:
1. If the option is supported on conversion/runtime level - it works.
- stream
- echo | no-echo
- unbuffered
- paged page-size(this option is converting and at runtime there are no page delimiters in our and real 4GL in Windows - we have the same result as on real system)
2. There is option that is not supported in Windows version of Progress environment
- map protermcap | no-map is not supported in Windows according to Progress documentation
3. There are options that we do not support on conversion level:
- convert (target trg_cp source src_cp) | no-convert - The experiments on Windows system with 4GL show this option should be supported in Windows.
- stream-handle option. According to OpenEdge 11.0 documentation this option should work the same way as stream option using appropriate handle corresponding to the stream value. However we do not convert this option(keyword stream-handle is unknown). Not sure if this is severe or not because as I can see this is just another way to use stream itself.

Is there any other work needed before you are done?

Is there any other work needed before you are done?

I would say yes if the DLL init-term procedure is completely OK. For now I do not see the things to do here. It is compiled with multithreading in mind and dynamically linked with MSVCRT.DLL and KERNEL32.DLL. Is this functionality good enough? Or we need more? For example - is it required to support old 32 bit Windows like Windows 2000 or NT may be? Or even systems that use Win32s old API?

Code Review

1. The approach is fine. Please remove all the logging code from the DllEntryPoint function. In addition, put comments in place of that logging.

For the DLL_PROCESS_ATTACH case:

// it is now safe to access the C runtime

For the DLL_PROCESS_DETACH case:

// final C runtime access must be done here

2. Are we sure that the MSVCRT is always present on Windows systems by default? If I understand correctly, we must distribute it with our app to be sure it is there AND we must be using MS Visual C++ (which we do NOT use).

3. Put comments in the appropriate code about not needing the signal processing on Windows.

4. Merge to the bzr rev 10355 level.

1. The approach is fine. Please remove all the logging code from the DllEntryPoint function. In addition, put comments in place of that logging.

OK.

2. Are we sure that the MSVCRT is always present on Windows systems by default? If I understand correctly, we must distribute it with our app to be sure it is there AND we must be using MS Visual C++ (which we do NOT use).

As far as I can see MSVCRT.DLL is a standard package at least from XP. Why do we need to distribute it? If the user removed the MSVCRT.DLL how can we guarantee the system is in a good enough shape in general to run the application(if one of the system part was removed, may be even accidentally)? We need to document the requirement, of course.

I have modified makefile to explicitly link with MSVCRT by override LDFLAGS+=-lmsvcrt ... option.

3. Put comments in the appropriate code about not needing the signal processing on Windows.

4. Merge to the bzr rev 10355 level.

OK.

As far as I can see MSVCRT.DLL is a standard package at least from XP. Why do we need to distribute it?

Are you sure it was not installed as part of some other application?

I have modified makefile to explicitly link with MSVCRT by override LDFLAGS+=-lmsvcrt ... option.

I understand.

Another question: is msvcrt.dll multithread-safe by default?

It looks good. Please get this runtime regression tested. I have applied the pending changes to staging and I am running conversion there under your userid. In 4.5 hours, it should be complete. You still have to apply your changes (evl_upd20130528a.zip) to /gc/convert/staging/p2j/ and rebuild. Then you can run regression testing in staging, directly.

Please get this runtime regression tested. I have applied the pending changes to staging and I am running conversion there under your userid. In 4.5 hours, it should be complete. You still have to apply your changes (evl_upd20130528a.zip) to /gc/convert/staging/p2j/ and rebuild. Then you can run regression testing in staging, directly.

OK.

Then you can run regression testing in staging, directly.

The testing completed. There are no regressions, results are 20130529_* - 20130530_* coped to the shared directory. The servers are now stopped.

Great! Please check it in and distribute it via email.

Please check it in and distribute it via email.

OK. One more question. There are the console logging code(printf() related calls) in process_win.c for info/debugging purpose. Can I left it in the file or I should remove this?

I should have caught that. No, that stuff MUST be removed. No debugging output should remain. If that is only in the windows code, then you don't need to regression test again. But you DO need to re-test the final windows process launching with your standalone testcases, to make sure that it wasn't broken when you remove the code.

Greg/Eugenie: if the client is ran in a non-TTY mode, the process launching will not work. Is this by intent? I think this line is at fault, in process_linux.c:114:

   // make sure we have a tty as stdin
   if (!isatty(STDIN_FILENO))
   {
      return -2;
   }

Greg/Eugenie: if the client is ran in a non-TTY mode, the process launching will not work. Is this by intent?

Yes, it is.

This part of code was transferred without changes from previous CHARVA Toolkit.c modifications. You can find the history of the changes(including this one I guess) looking at CHARVA modified Toolkit.c source.

The 4GL process launching requires a controlling terminal or pty. For instance, the programs that are launched from MAJIC will not work otherwise. This is why we have our JNI process launching code instead of relying upon the Java process launching support.