FWD » Core Development » Base Language

Proper processing of Client Platform Dependencies requires these features to be implemented in a separate client JVM process.

However, there are definitely cases in which these same features could be implemented in a purely server-side manner and converted code could uses them within the FWD JVM without a problem. For this purpose, we will enable usage of some of the client platform features from within the server process. Where this can be safely done, it may represent a major improvement in performance.

One can easily consider these cases (maybe others too):

• file system access and streams
• XML to/from file
• web services and sockets usage

We would implement 4GL syntax support for forcing the processing to be server side. Some minor conversion and runtime changes are needed, but most of the support would easily move to the server side without much effort.

The only other issue here is that we need to implement a security manager plugin to secure access to the server side resources. At least the filesystem resources need to be protected this way. Otherwise there might be major unintended access for users to data that should be secured on a server but now might be exposed.

See also some discussion in #3254-4.

A key part of the solution will be to implement a server-side object instead of the normal client-side remote proxy that is normally used. This will allow transparent usage of the server side without any other changes.

That server-side object would need to have the security manager checks built in.

For those client dependencies which cannot and should not be implemented, we can implement a kind of mock implementation that does nothing. This would "null out" any usage safely.

I'd like to get an estimate of the performance improvement this task can offer for non-interactive use cases. Perhaps the JMX instrumentation of the DAP can be used to make an estimate?

On the other hand, the effort to do a quick implementation (without a security manager plugin) is probably not too bad. Doing a first pass would be even more effective to see the results.

The implementation of #4065-5 (without security) seems pretty straightforward. The less obvious part will be eliminating any ThinClient or other client-specific code dependencies from the backing workers so that they can run on either the client or server.

• do we want to have a hybrid model where some parts exist on the client? If yes, then the client JVM process is still needed. The only useful case I see for a hybrid model is to have interactive clients which allocate the memptrs, open client sockets, call web services, on the server-side, and not on the client-side. File access is more tricky to move to server-side, as we need the security plugins to 'sandbox' them. And something I haven't considered until now - server sockets can't be opened by multiple clients if they ran on the same machine. This may be considered a limitation, which can be partially solved with remote brokers to launch the client on a different machine.
• if we don't need a hybrid approach, my first thought is to have a mode similar to the process:arch:single=true, where the client is ran as a thread in the server's JVM. But currently this is broken, as SessionManager.get().isLeaf() will return false, becauses the SessionManager.instance is a JVM-wide singleton, set to a RouterSessionManager instance. Regardless if the client APIs are executed in a different thread or in the same Conversation thread on the server-side, we will need some changes in client-side classes which rely on isLeaf() to determine if is the client or server using it.
• the hybrid approach also has some other complexities, where we need to identify client-side resources which now are implemented on the server-side, and what are other client-side dependencies which still need access to those resources (which now exist on server-side). Think memptr, are there cases where some client API has a memptr parameter? If so, these APIs will need to have a 'reverse-proxy' back to the server, to access it, or otherwise refactor them to execute on server-side.
• you mention a 'server-side object'. We already have RemoteObject.obtainLocalInstance to obtain a 'local proxy'. What would be different for this 'server-side object'?
• batch/appserver clients started via the FWD server (and not manually or via a remote broker client) can be marked to use this 'server-side mode' always.

do we want to have a hybrid model where some parts exist on the client? If yes, then the client JVM process is still needed. The only useful case I see for a hybrid model is to have interactive clients which allocate the memptrs, open client sockets, call web services, on the server-side, and not on the client-side.

We may need a hybrid model for future requirements. I don't expect the REST cases to require it, so it may be best to do this in 2 phases with the hybrid model left for a 2nd phase.

I do see the hybrid model as being needed for more than just interactive cases. All it takes is one shared library that is not thread-safe, or a case where a server socket must listen on the same port for more than one session... this can be as likely for non-interactive code as for interactive.

if we don't need a hybrid approach, my first thought is to have a mode similar to the process:arch:single=true, where the client is ran as a thread in the server's JVM.

I don't see this as a good solution. This must coexist in a system where real client JVMs still operate. In other words, this should be a session level configuration.

BTW, I think we would be better off removing the process:arch:single=true mode. We never use it and it just complicates the code. We don't have to do it here, but then again we'll be editing the same locations anyway.

you mention a 'server-side object'. We already have RemoteObject.obtainLocalInstance to obtain a 'local proxy'. What would be different for this 'server-side object'?

Yes, this is the idea. The point is that we must instantiate the equivalent of MemoryDaemon, StreamDaemon, FileSystemDaemon... on the server side and use those instead of the remote version in those sessions which are configured.

batch/appserver clients started via the FWD server (and not manually or via a remote broker client) can be marked to use this 'server-side mode' always.

It should probably be optional so that we have a "sure to be compatible" option. Perhaps we make this the default but allow the session to be configured to use the spawner and create a client JVM if so configured.

I have some changes which brings memptr fully on server-side (very pin-pointed changes, no major refactoring).

• with memptr fully on server-side, a copy-lob or any other statement which uses a memptr and another client-side resource, now will have to call-back on the server-side to access the memptr.
• server-side will need to initialize network proxies for MemoryDaemon and LibraryDaemon (here what I really need is the libp2j.so loading from LibraryDaemon, I think I'll move that in a separate method to be able to load it explicitly).
• LeafSessionManager.getSession() needs to return non-null, so activeSession is set in connectDirect. This required some small patches in some classes which created a network object only if session != null, instead of relying on SessionManager.isLeaf(). Also, this required RemoteObject.obtainInstance to first look on a local instance and fallback to network server only if there is no local instance.
• how should we enable the server-side memptr? Via a directory configuration? A bootstrap configuration? Should we allow 'pick and choose' which resources are on server-side and which are on client-side? This affects how the client-side can access this resource, if is moved on server.

I assume (for now) that you are not removing the ability to use memptr on the client. Perhaps we will find that long term it can be server only. To do so would require changes to ensure that client side usage of copy-lob and library calls have their memptr requirements met dynamically.

• with memptr fully on server-side, a copy-lob or any other statement which uses a memptr and another client-side resource, now will have to call-back on the server-side to access the memptr.

Please mark those APIs (with TODOs) which need re-optimization. Where possible, we will want to rework the downcalls in these cases to pass the data needed.

• server-side will need to initialize network proxies for MemoryDaemon and LibraryDaemon (here what I really need is the libp2j.so loading from LibraryDaemon, I think I'll move that in a separate method to be able to load it explicitly).

Yes, that makes sense.

I think we have to add some memptr code for this server-side case to limit access to pointers which were allocated for a different session. In other words, it is a security breach if some code tries to access arbitrary memory locations. This is possible using SET-POINTER-VALUE() which is very dangerous on the server side. We need some kind of per-session registry of valid memory regions (base address, size). This would hold regions directly allocated by the session.

I think we have a problem with memory allocated by a library call. This can happen with output parameters, for which we can dynamically setup a region. The problem is for library calls that return an address (an integer or int64) in a structure where that address represents a pointer to memory that has been allocated. In such cases the memory region is available in process, but we don't know about it. SET-POINTER-VALUE() must be to be disallowed by default on the server unless it is using an address in our list of known valid regions.

If customers have code that breaks from this restriction, we can provide a mechanism to configure a bypass for specific code. I would wait to implement that additional feature until we find a customer that needs it.

Eugenie: Please do a code review of our current libp2j implementation to try to find any issues related to thread safety. I'm concerned that we have not written the code there to gracefully/correctly be executed on the server in multuple threads. The signal processing and pseudo-terminal processing comes to mind for me, as needing some update.

• LeafSessionManager.getSession() needs to return non-null, so activeSession is set in connectDirect. This required some small patches in some classes which created a network object only if session != null, instead of relying on SessionManager.isLeaf(). Also, this required RemoteObject.obtainInstance to first look on a local instance and fallback to network server only if there is no local instance.

OK. I'm also OK if we want to eliminate the "single session" mode. We never use it and it is a complication that should probably be removed.

• how should we enable the server-side memptr? Via a directory configuration? A bootstrap configuration? Should we allow 'pick and choose' which resources are on server-side and which are on client-side? This affects how the client-side can access this resource, if is moved on server.

This cannot be a bootstrap configuration thing, as that would open a security hole (any client could set this and open up access on the server side). This must be configured in the directory. I think it is enough for now that the usage is activated on a user/group/server/default basis. Later, we may need some granularity by resource to have some things on the client and others on the server, but for now let's assume it is all one way or the other.

Greg Shah wrote:

I assume (for now) that you are not removing the ability to use memptr on the client. Perhaps we will find that long term it can be server only. To do so would require changes to ensure that client side usage of copy-lob and library calls have their memptr requirements met dynamically.

Exactly, when memptr is on server-side, client-side will need to call the server for any memptr usage.

• with memptr fully on server-side, a copy-lob or any other statement which uses a memptr and another client-side resource, now will have to call-back on the server-side to access the memptr.

Please mark those APIs (with TODOs) which need re-optimization. Where possible, we will want to rework the downcalls in these cases to pass the data needed.

OK

I think we have to add some memptr code for this server-side case to limit access to pointers which were allocated for a different session. In other words, it is a security breach if some code tries to access arbitrary memory locations. This is possible using SET-POINTER-VALUE() which is very dangerous on the server side. We need some kind of per-session registry of valid memory regions (base address, size). This would hold regions directly allocated by the session.

I think we have a problem with memory allocated by a library call. This can happen with output parameters, for which we can dynamically setup a region. The problem is for library calls that return an address (an integer or int64) in a structure where that address represents a pointer to memory that has been allocated. In such cases the memory region is available in process, but we don't know about it. SET-POINTER-VALUE() must be to be disallowed by default on the server unless it is using an address in our list of known valid regions.

Good points, I'll add them.

This cannot be a bootstrap configuration thing, as that would open a security hole (any client could set this and open up access on the server side). This must be configured in the directory. I think it is enough for now that the usage is activated on a user/group/server/default basis. Later, we may need some granularity by resource to have some things on the client and others on the server, but for now let's assume it is all one way or the other.

Understood, thank you.

• should I add support for server-side LibraryDaemon together with memptr? They are tightly coupled in the p2j.library package.
• I'm using RoaringBitmap to keep the 'in use' address space for each client context, on server-side.
• the directory configuration can be named server-side-resources and keep a list of FWD qualified class names, like com.goldencode.p2j.util.memptr - or do you want something more user-friendly?

Also, is it enough to raise an ERROR condition if the application code tries a set-pointer-value() to something outside the context's known address space?

should I add support for server-side LibraryDaemon together with memptr? They are tightly coupled in the p2j.library package.

Yes

do you want something more user-friendly?

A single string with comma separated values will keep it simple.

                <node class="string" name="server-side-resources">
                  <node-attribute name="value" value="memptr,library,streams"/>
                </node>

The names don't have to match classes, they should just make sense for the feature.

is it enough to raise an ERROR condition if the application code tries a set-pointer-value() to something outside the context's known address space?

Yes, I think that is OK for now. Make sure that we log it too, since these are places that need some attention.

The code is complete for memptr and library, I need to finish a testing of #6075 and some other scenarios.

I've added a OSResourceManager class with two sub-classes, ClientSideResourceManager and ServerSideResourceManager. This contains APIs to be used on client, server or common client-server code, to get the proper network or local instance, or to initialize the resource properly (either on client or server-side, by creating the network or local proxy).

Code Review Task branch 3821c Revision 13885

Overall, I very much like the changes.

1. I don't fully understand the use of statics in OSResourceManager (and subclasses). Its data seems like it will always contain global state across the server or client. In other words, on the server, there is no per-session capability. In addition, it is not thread-safe when used in this way. Am I mis-understanding this?

2. In regard to the thread-safety of OSResourceManager (and subclasses), even if we make this per-context we should eventually implement this using synchronization. It is likely that it will be called in a multi-threaded environment and I would prefer if it will be safe. If we don't implement this now, then we need to remember it is technical debt. Perhaps we need some TODO comments to this effect.

3. In the static initializer for OSResourceManager, the resource names should be processed case-insensitively.

4. Any usage of streams/sockets which remains client-side and needs memptr access will be broken now, right? I don't see any rework to up-call or to pass the data down more efficiently.

Roger: As part of rev 13885, the RoaringBitmap library version has changed from 0.9.0 to 0.9.27. Would you please handle the version number changes in our documentation/license project?

Greg Shah wrote:

1. I don't fully understand the use of statics in OSResourceManager (and subclasses). Its data seems like it will always contain global state across the server or client. In other words, on the server, there is no per-session capability. In addition, it is not thread-safe when used in this way. Am I mis-understanding this?

There is no context-local state to be kept. These maps are used only when initializing the ClientSideResourceManager or ServerSideResourceManager, which is done only once per JVM (at server or client startup). Every other access is read-only.

But I see what you mean now, the code is not working properly on a 'per-user' basis... I need to fix this.

3. In the static initializer for OSResourceManager, the resource names should be processed case-insensitively.

The server-side-resources string is lowercased before splitting it into tokens.

4. Any usage of streams/sockets which remains client-side and needs memptr access will be broken now, right? I don't see any rework to up-call or to pass the data down more efficiently.

No, is not broken, client-side will call into server-side, there are TODOs in place where optimizations may be needed.

I've fixed the per-context support of server-side-resources in 3821c/13886.

Code Review Task Branch 3821c Revision 13886

It looks good.

Greg Shah wrote:

Roger: As part of rev 13885, the RoaringBitmap library version has changed from 0.9.0 to 0.9.27. Would you please handle the version number changes in our documentation/license project?

Sure thing.

Roger Borrello wrote:

Greg Shah wrote:

Roger: As part of rev 13885, the RoaringBitmap library version has changed from 0.9.0 to 0.9.27. Would you please handle the version number changes in our documentation/license project?

Sure thing.

Done. Please rename RoaringBitmap-0.9.0.jar_LICENSE.txt to RoaringBitmap-0.9.27.jar_LICENSE.txt on the web server: https://proj.goldencode.com/artifacts/licenses/

Please rename RoaringBitmap-0.9.0.jar_LICENSE.txt to RoaringBitmap-0.9.27.jar_LICENSE.txt on the web server: https://proj.goldencode.com/artifacts/licenses/

Done.

There is an issue with server-side memptr which affects #6277. The problem is related to memptr used at define return parameter, define output parameter at the native call.

For DEFINE RETURN, this fails at LibraryManager.invoke:331:

      // copy back return value
      if (retval != null)
      {
         retval.restore(retaddr);
      }

with Memory violation: trying to SET-POINTER-VALUE to an address unallocated in this context.:

memptr.checkAddressSpace(long) line: 1497    
memptr.setPointerValue(long) line: 506    
NativeBuffer.restore(long) line: 257    
LibraryManager.invoke(String, String, int, boolean, CallingConvention, Signature) line: 331    

I see memptr.setPointerValue is used at:

NativeBuffer.render(long)
NativeBuffer.restore(long)
NativePrimitive.render(long)
NativePrimitive.restore(long)
NativeString.render(long)
NativeTypeArray.render(long)
NativeTypeArray.restore(long)

An approach would be to force registration of this pointer in the FWD context's address space (as it was returned by an API call), in the cases above. But the deregistration of this pointer from the FWD context's address space may not happen, if the memptr is not deallocated in the Java code, but in another native library call...

Another approach would be to allow memptr to be assigned addresses returned by the native API calls, and not register them in the FWD context's address space, when server-side memptr is used.

But the deregistration of this pointer from the FWD context's address space may not happen, if the memptr is not deallocated in the Java code, but in another native library call...

This is a common case. Native libraries often allocate memory and return a pointer. This allocation could be done via CRT functions, OS functions or other means. The caller must not attempt to deallocate such pointers.

An approach would be to force registration of this pointer in the FWD context's address space (as it was returned by an API call), in the cases above.

This may make sense. The biggest issue is that the size of the memory area is unknown to us.

Another approach would be to allow memptr to be assigned addresses returned by the native API calls, and not register them in the FWD context's address space, when server-side memptr is used.

I think we must make an effort to avoid protection faults since such a problem will crash the server JVM. If we have no registration or protection, how do we avoid such crashes?

Perhaps we should require that the 4GL code is modified to tell us the size of the returned pointer when SET-POINTER-VALUE is called. This is something that would be case by case (it depends on the native library call itself and sometimes may be based on data structure size and other times on some size value that is passed in). Either way, the caller will know this information and we cannot deduce it ourselves. With that information, I think we can protect the server.

Greg Shah wrote:

Perhaps we should require that the 4GL code is modified to tell us the size of the returned pointer when SET-POINTER-VALUE is called. This is something that would be case by case (it depends on the native library call itself and sometimes may be based on data structure size and other times on some size value that is passed in). Either way, the caller will know this information and we cannot deduce it ourselves. With that information, I think we can protect the server.

For this specific case, is not about 4GL SET-POINTER-VALUE - is about internal FWD assignment of the memptr pointer, as returned via a RETURN or OUTPUT parameter.

Currently, when SET-POINTER-VALUE is called, any address in the context's allocated address space is allowed. We don't reserve address space on SET-POINTER-VALUE.

And you raise a good point - there is no knowledge about the size of the address from RETURN or OUTPUT params at the native call, so I can't allocate this in the FWD context's address space.

The only solution I see is to disable the address space check for RETURN and OUTPUT parameters, in the FWD runtime.

The only solution I see is to disable the address space check for RETURN and OUTPUT parameters, in the FWD runtime.

I don't think we can do this. It is important that each application has been carefully vetted to ensure it is safe for server-side resource usage. Dangerous operations should be reviewed to confirm. If that requires 4GL changes to inform FWD of how to make this safe, it is OK with me.

In this case, we can augment the RETURN or OUTPUT parameter with additional size information. Actually it is probably better to add a call to register the externally allocated pointer (and size of the memory allocation) in FWD. By making this explicit, we tell FWD that it is OK to allow access to that range.

Greg Shah wrote:

In this case, we can augment the RETURN or OUTPUT parameter with additional size information. Actually it is probably better to add a call to register the externally allocated pointer (and size of the memory allocation) in FWD. By making this explicit, we tell FWD that it is OK to allow access to that range.

I understand, but I have at least a case in #6277 where the RETURN parameter is not being deallocated on the FWD server side, I assume is being deallocated by the native library call (in this case, the library call is https://www.openssl.org/docs/man1.1.1/man3/EVP_sha1.html). My point is if the memory is not deallocated on FWD server, this will lead to a memory leak in the FWD's allocated address space. So this needs to be solved, too, somehow - maybe directly in the 4GL code?

Constantin Asofiei wrote:

... in this case, the library call is https://www.openssl.org/docs/man1.1.1/man3/EVP_sha1.html

I mean this is the library call which returns the pointer to FWD, and after that the pointer is set as first argument to https://www.openssl.org/docs/man1.1.1/man3/HMAC.html

I understand, but I have at least a case in #6277 where the RETURN parameter is not being deallocated on the FWD server side, I assume is being deallocated by the native library call (in this case

This is the common case for return and output pointers.

My point is if the memory is not deallocated on FWD server, this will lead to a memory leak in the FWD's allocated address space. So this needs to be solved, too, somehow - maybe directly in the 4GL code?

This is not something we can handle in FWD. If there is a memory leak by using a native library, then that leak will exist in both FWD and in OpenEdge. Although we would like there to be no leak, it is not required that we resolve this. I'm more worried about avoiding protection violations.

Greg Shah wrote:

My point is if the memory is not deallocated on FWD server, this will lead to a memory leak in the FWD's allocated address space. So this needs to be solved, too, somehow - maybe directly in the 4GL code?

This is not something we can handle in FWD. If there is a memory leak by using a native library, then that leak will exist in both FWD and in OpenEdge. Although we would like there to be no leak, it is not required that we resolve this.

Here I mean if the memory is allocated and deallocated by a native library call, FWD will keep it allocated in the context's address space, as it 'allocated' this address space when the RETURN or OUTPUT parameter was used - so this is a FWD leak, not an application leak.

I'm more worried about avoiding protection violations.

Understood.

FWD will keep it allocated in the context's address space, as it 'allocated' this address space when the RETURN or OUTPUT parameter was used - so this is a FWD leak, not an application leak.

We don't actually allocate any additional memory, but there will be an extra entry in our registry. Is that what you mean?

Greg Shah wrote:

We don't actually allocate any additional memory, but there will be an extra entry in our registry. Is that what you mean?

Exactly. FWD's allocated address space registry will end up have unallocated memory marked as allocated.

Yes, this is a flaw but it is better than the alternative.

Can we detect when the same lvalue is re-assigned a new pointer value (on return or output) and automatically deregister at that time? That may lessen the chances of a significant leak.

I'm currently working on the file-system server-side processing. Here are a few notables:

• Current FWD security model doesn't have primitives for file-system resources, this will have to be extended.
• The file system security plugin (FileSystemResource) will be based on BitFlagsResource.
• The actual file-system security model will be based on the Linux file-system security model.
• File system resources names will be defined as absolute file names. With the option to specify a first-level subdir (*) or any-level subdirs (**).
• The security plugin will be in effect only on server-side, not on the clients.

Are the above reasonable?

I created task branch 4065a based on trunk revision 14477.

Current FWD security model doesn't have primitives for file-system resources, this will have to be extended.
The file system security plugin (FileSystemResource) will be based on BitFlagsResource.

Yes on both.

The actual file-system security model will be based on the Linux file-system security model.

Can you clarify what you mean here? The BitFlagsResource has flags CDRWN (create, delete, read, write, denied) and these are the correct permissions that we should support. The Linux file system permissions are too weak in comparison. Likewise, we don't need to implement the concept of owner, group, others because our security model defines a much richer set of possible cases. Any number of accounts including processes/servers, groups and others can all be defined and working together. I don't want to limit us to a Linux approach. That is not a good model.

I'm not a fan of the Windows drive letter either, but I think we cannot avoid dealing with it. I hope we can hide some of that system-specific nonsense behind some core J2SE classes like File.

File system resources names will be defined as absolute file names.

I don't want to limit us to absolute filenames. In fact, I think it would be a bad practice to encode these because moving the server location (current directory) would then require potentially major changes to the directory. When one considers that a single converted application will be installed in potentially thousands of different locations, having to manage absolute filenames will be painful.

If a filename starts with / or a drive letter + colon, then it is absolute. Otherwise it is considered relative to the current directory. This will allow us to support fully relative projects, where some portion of the server file system is opened up for access but the rest is inaccessible.

With the option to specify a first-level subdir () or any-level subdirs (*).

Good. We probably should support the other normal "globbing" character ?.

The security plugin will be in effect only on server-side, not on the clients.

Yes

Greg Shah wrote:

The actual file-system security model will be based on the Linux file-system security model.

Can you clarify what you mean here?

I was referring to the simplicity of bitset values set on the file system objects. The actual set of permissions is as you say. Except I think we should implicitly deny access when no permission rule is matched. With the above I'm not sure what should be the semantic of the DENIED permission. Also we should introduce the EXECUTE permission.

File system resources names will be defined as absolute file names.

I don't want to limit us to absolute filenames. In fact, I think it would be a bad practice to encode these because moving the server location (current directory) would then require potentially major changes to the directory. When one considers that a single converted application will be installed in potentially thousands of different locations, having to manage absolute filenames will be painful.

If a filename starts with / or a drive letter + colon, then it is absolute. Otherwise it is considered relative to the current directory. This will allow us to support fully relative projects, where some portion of the server file system is opened up for access but the rest is inaccessible.

OK.

With the option to specify a first-level subdir () or any-level subdirs (*).

Good. We probably should support the other normal "globbing" character ?.

OK.

Hynek Cihlar wrote:

Also we should introduce the EXECUTE permission.

I will rephrase this to a question. Should we pursue some kind of EXECUTE permissions on the OS-COMMAND statement?

Except I think we should implicitly deny access when no permission rule is matched.

Agreed. We always should assume no access unless explicitly granted.

With the above I'm not sure what should be the semantic of the DENIED permission.

It is still needed in the case where existing rules would otherwise grant access. In this way you can exclude items from a larger set of included items.

Also we should introduce the EXECUTE permission.

I like this, yes.

Should we pursue some kind of EXECUTE permissions on the OS-COMMAND statement?

It is a really good idea.

I think you can add this EXECUTE extra bit into the BitFlagsResource directory.

Greg Shah wrote:

Should we pursue some kind of EXECUTE permissions on the OS-COMMAND statement?

It is a really good idea.

The question is how to enforce the permission. The command may be any expression interpreted by the system. We could just do a regexp match on the value.

Should we pursue some kind of EXECUTE permissions on the OS-COMMAND statement?

It is a really good idea.

The question is how to enforce the permission. The command may be any expression interpreted by the system. We could just do a regexp match on the value.

I think we focus on the command itself and ignore any parameters. The code in ProcessOps will have the evaluated expression that includes the command itself as the leftmost text. At that time we can lookup the command to ensure that we have EXECUTE access before allowing the OS-COMMAND to continue. Treat it like any other relative/absolute filename. If it matches with a rule that includes EXECUTE, then it can run. Otherwise we raise an ERROR condition.

Hynek Cihlar wrote:

I created task branch 4065a based on trunk revision 14477.

FYI, trunk is on rev 14483.

Constantin Asofiei wrote:

Hynek Cihlar wrote:

I created task branch 4065a based on trunk revision 14477.

FYI, trunk is on rev 14483.

Thanks for the correction. My local branch is older, 4065a is indeed branched off 14483.

Greg, do we want to interpolate the file paths with a set of known variables? Like logged in user, application name, working dir, etc? This would make the system easier to manage.

Hynek Cihlar wrote:

Greg, do we want to interpolate the file paths with a set of known variables? Like logged in user, application name, working dir, etc? This would make the system easier to manage.

Yes, that makes sense. This is pretty easy to do in the SecurityManager plugin.

Greg, I found several places in the client UI where file IO is performed directly. An example is a button widget loading an image.

With respect to the anticipated use cases does it make sense to replace these with the managed file system resource calls?

With respect to the anticipated use cases does it make sense to replace these with the managed file system resource calls?

Yes.

Doing it that way would also allow us to intercept and redirect resources from the jar in a cleaner way. Today we've inserted some "one off" ways to load resources from the jar but really it would be better as an architected and standard feature.

I rebased 4065a against trunk revision 14520.

From Hynek via email:

Changing the direct IO in the client process to go through a remoted object on the server will make the performance worse. Or do I miss something?

Please describe what this refers to. You mean resources which are required by the client-side (like images for GUI and stuff like that)?

Constantin Asofiei wrote:

From Hynek via email:

Changing the direct IO in the client process to go through a remoted object on the server will make the performance worse. Or do I miss something?

Please describe what this refers to. You mean resources which are required by the client-side (like images for GUI and stuff like that)?

Yes, I meant resources needed by the client side.

• there is EDITOR:READ-FILE, which can't be on the server-side, as the FWD client will need to go over the network to read this.
• other usage like image loading, etc, which are on the client-side only.

Constantin Asofiei wrote:

Greg, correct me if I'm wrong, but server-side processing is required only by legacy streams (used by INPUT/OUTPUT statements) and other cases where the stream doesn't interact with the FWD Client; for example:

• there is EDITOR:READ-FILE, which can't be on the server-side, as the FWD client will need to go over the network to read this.
• other usage like image loading, etc, which are on the client-side only.

Constantin, see #4065-59 and the related where we discussed this.

Hynek Cihlar wrote:

Constantin, see #4065-59 and the related where we discussed this.

OK, but when server-side streams are active, the client should not call the server-side and read the file, over the socket, if this is a 'client-only' file. It will add unnecessary overhead IMO.

Constantin Asofiei wrote:

Hynek Cihlar wrote:

Constantin, see #4065-59 and the related where we discussed this.

OK, but when server-side streams are active, the client should not call the server-side and read the file, over the socket, if this is a 'client-only' file. It will add unnecessary overhead IMO.

This was my concern, too and this why I wanted to descope it.

I have a question about this task... In #4938-52 I need to configure the web client such that we have this in the directory:

    <node class="container" name="server">
      <node class="container" name="default">
        <node class="container" name="webClient">
          <node class="string" name="configFile">
            <node-attribute name="value" value="user_client.xml"/>
          </node>
          <node class="string" name="workingDir">
            <node-attribute name="value" value="~/.app-cfg"/>
          </node>

We'd need the ~ to be resolved to the client's homespace (not the server's) so that we can load cmd-line-option ininame= from a /home/<user>/.app-cfg/user_client.xml, which the application can modify based upon user preference. Using the tilde allows the same directory to work for any user accessing from a web browser and signing into the OS. Perhaps $HOME could be used, but the point is, this needs to come from the client's filesystem.

Should this be working already, without changes going into this task? Because I see the value being read at server startup (I temporarily modified the code to handle tilde) so when the web-client starts, it is already using a value determined on the server's filesystem.

Roger Borrello wrote:

I have a question about this task... In #4938-52 I need to configure the web client such that we have this in the directory:
[...]

We'd need the ~ to be resolved to the client's homespace (not the server's) so that we can load cmd-line-option ininame= from a /home/<user>/.app-cfg/user_client.xml, which the application can modify based upon user preference. Using the tilde allows the same directory to work for any user accessing from a web browser and signing into the OS. Perhaps $HOME could be used, but the point is, this needs to come from the client's filesystem.

Should this be working already, without changes going into this task? Because I see the value being read at server startup (I temporarily modified the code to handle tilde) so when the web-client starts, it is already using a value determined on the server's filesystem.

Currently we don't interpolate the workingDir directory value, also I don't think you want that. To me it looks the resolution of the ini file itself should be enhanced regardless of workingDir.

If we look at how the options in the ini file (and the registry) are resolved, we should probably also consider to allow the ini values resolution to check a user specific location. OE checks the current user registry hive (HKCU) and then local machine (HCLM) Windows registry hive when resolving a specific variable. If we follow this logic, then we should consider a user specific location of the client process system account (home?) before resolving the ini file against the working dir. Also the options from the user-specific dir and working dir should be merged with the user-specific taking precedence. This is because OE does this resolution per value.

Greg, what do you think?

but server-side processing is required only by legacy streams (used by INPUT/OUTPUT statements) and other cases where the stream doesn't interact with the FWD Client

Correct

but when server-side streams are active, the client should not call the server-side and read the file, over the socket, if this is a 'client-only' file. It will add unnecessary overhead IMO.

This was my concern, too and this why I wanted to descope it.

This was never in scope, so it doesn't need to be a separate task. We aren't doing that.

what do you think?

I think this is a discussion that is unrelated to this task. Yes, we probably have a bug in how we resolve the web client working dir. That has nothing to do with this task.

As far as the INI/registry support, we can discuss that elsewhere. We'll have to write some code to check our compatibility and then fix any issues. Roger's problem doesn't relate to the INI/registry support. It is really something to discuss and fix in #4938. In that task we have to load a per-user bootstrap config file located in the home dir and he has run across a bug.

From email:

Hynek wrote:

I meant the loading resources for the UI (icons, bitmaps). I got your point about making the process more concise, but the think is it will degrade performance. So the question is whether we should descope this from #4065, which is primarily performance-focused.

Greg wrote:

The idea here is that we find them on the server side before we ever call down to the client side. It is not an upcall. We already do this in at least some cases, but it is not consistent with the rest of your code. Each place is just a patch done at some point rather than a designed approach.

Hynek wrote:

Can you give an example of the code that already does this?

See anything that calls LT.getResourceStreamFromApplication(String) or LT.getImageStreamFromApplication(String image).

The server-side shell execution will require more thought. For the file system it is straightforward to get a canonical path of the resource and check with the predefined ACLs. However for the shell command execution, resolving the canonical command (taking into account the environment, paths, etc.) is not trivial. The canonical representation of the executed command is necessary to avoid any ambiguities and avoid potential security holes.

In regard to the shell commands, I think it is OK if we require that the commands meet one of these criteria:

• It is an absolute path. Any such path can be canonicalized by our code and compared safely. OR
• It is a command with no pathing information. In such a case we would set a "secure" PATH from which that command can be safely found. That secure path would be configured in our directory.

Greg/Hynek: for server-side memptrs, we still have the problem when the memory address is incoming from a OS library call - as this is not tracked in the memptr$WorkArea.addressSpace (this contains only the addresses allocated via memptr.setLength).

We already have an application where server-side memptrs can't be used because of this limitation.

When a client process accesses the server, how do you know if its a local client (on the same host) or a remote one with this approach?

Take a look at the library manager support (or memptr) in trunk.

Thank you for the help. I'll find the configs now on trunk.

There is no email merge notification for task branch 4065 and no comment which revision it was on trunk. How do I find it?

Galya Bogdanova wrote:

There is no email merge notification for task branch 4065 and no comment which revision it was on trunk. How do I find it?

Please see #6075-25 for the main commit.

The initial support was put in as branch 3821c revision 13885/13886.

Additional support is written in 4065a but has not been merged to trunk yet as it needs review. But you can look at the existing code for the model.

Greg Shah wrote:

But you can look at the existing code for the model.

If I don't know the revision, how do I know what model we're speaking about.

Galya Bogdanova wrote:

Greg Shah wrote:

But you can look at the existing code for the model.

If I don't know the revision, how do I know what model we're speaking about.

When a 3821c revision is mentioned, that is a trunk revision. 3821c became trunk some time ago.

Constantin Asofiei wrote:

When a 3821c revision is mentioned, that is a trunk revision. 3821c became trunk some time ago.

So on trunk there is a model of something in a revision that was rebased, its number is lost. There is no wiki, but comments in a different task. I should start investigating from there.

Greg Shah wrote:

The initial support was put in as branch 3821c revision 13885/13886.

Ah, I see... it should be trunk 13885/13886.

Code Review Task Branch 4065a Revisions 14521 through 14527

Overall this is a really nice update! I think the approach is quite elegant. I'm happy that the implementation was done with a relatively small amount of change for the features provided.

1. BitFlagsResource.getRightsInstance() returns FileSystemRights instances instead of BitFlagsRights instances. This doesn't seem right.

2. In FileSystemDaemon.deleteWorker(), did you place the delete rights check after the recursion on purpose? Is the idea that the content may be deletable when the target is not deletable? My initial thought is that if you can't delete the target then you shouldn't have the chance to delete the contents, but I may be thinking incorrectly in this case. My sense is that the current result will be non-intuitive. In security-related matters, non-intuitive often means a security hole. If we're going to allow this we may want to check read access as well otherwise don't allow the recursion but I would go with just a top-level denial instead.

3. In regard to the StreamWrapper.assign() TODO, the protection code was put in with this commit:

revno: 11328.1.13
author: Hynek Cihlar <hc@goldencode.com>
committer: Constantin Asofiei <ca@goldencode.com>
branch nick: 4124a
timestamp: Tue 2019-07-16 13:37:01 +0200
message:
  Reworked the stream-by-name resolution. Now it all happens on the server, also the conversion changes were reverted.

  Refs #3751.

I don't know why it was done but I guess we have some case where we use something other than a RemoteStream on the server side and adding it is "no bueno". This deserves some additional consideration before merge.

4. GenericFrame.getRemoteStreamId() has dependencies on using RemoteStream. How do we handle that? The UI code really does need to stay on the client and redirected terminal stuff will need to be there too. If we have code that uses that, do we need some protection logic to ensure it fails safely when in server-side file system mode?

5. I prefer not to add dependencies in BitFlasgResource for edu.emory.mathcs.backport.java.util.*. It is not an approved 3rd party library.

6. DirStream should not have an import for org.apache.tinkerpop.gremlin.process.traversal.

7. In FileSystem, please add public to the new additions.

Constantin: Please review.

Greg Shah wrote:

Code Review Task Branch 4065a Revisions 14521 through 14527

Overall this is a really nice update! I think the approach is quite elegant. I'm happy that the implementation was done with a relatively small amount of change for the features provided.

1. BitFlagsResource.getRightsInstance() returns FileSystemRights instances instead of BitFlagsRights instances. This doesn't seem right.

True, this as unexpected change. I will fix this.

2. In FileSystemDaemon.deleteWorker(), did you place the delete rights check after the recursion on purpose? Is the idea that the content may be deletable when the target is not deletable? My initial thought is that if you can't delete the target then you shouldn't have the chance to delete the contents, but I may be thinking incorrectly in this case. My sense is that the current result will be non-intuitive. In security-related matters, non-intuitive often means a security hole. If we're going to allow this we may want to check read access as well otherwise don't allow the recursion but I would go with just a top-level denial instead.

Yes, the idea was "the content may be deletable while the target not". But I agree this is very counter-intuitive and will change this according your proposal - a top-level denial.

3. In regard to the StreamWrapper.assign() TODO, the protection code was put in with this commit:

[...]

I don't know why it was done but I guess we have some case where we use something other than a RemoteStream on the server side and adding it is "no bueno". This deserves some additional consideration before merge.

I'm not sure why I removed the check in the first place. I think this was needed at some point as the implementation evolved. I'll put the check back, the wrapper can handle remote and non-remote streams without problem.

4. GenericFrame.getRemoteStreamId() has dependencies on using RemoteStream. How do we handle that? The UI code really does need to stay on the client and redirected terminal stuff will need to be there too. If we have code that uses that, do we need some protection logic to ensure it fails safely when in server-side file system mode?

Since only file and dir streams are considered to be directly created on the server, anything else will work as ususal. This will include anything that needs to run on the client, like print and terminal streams.

5. I prefer not to add dependencies in BitFlasgResource for edu.emory.mathcs.backport.java.util.*. It is not an approved 3rd party library.

These were not needed, added by the IDE. Will remove.

6. DirStream should not have an import for org.apache.tinkerpop.gremlin.process.traversal.

Same as above.

7. In FileSystem, please add public to the new additions.

Will do.

• ConcurrentResource.resourceTypeName - the javadoc was indented right
• HighLevelObject.LOG - we are replacing java.util.logging in #5703, just something to keep track in #5703 or if #5703 gets merged before this
• LeafSessionManager.java, BitFlagsConstants.java, LogHelper.java - there is no change beside the end-of-file newline, this can be reverted
• ImportWorker has import com.goldencode.p2j.persist.pl.Functions - is this needed?
• BitFlagsResource - there is import edu.emory.mathcs.backport.java.util.* and import @edu.emory.mathcs.backport.java.util.Arrays;
• BitFlagsResource.isRightsSetValid - I don't understand this code:
  

         // check extended bits if any
           if (!set &&  extBits != null)
           {
              for (int i = 4; i < Math.min(3, extBits.length); i++)
              {
  

  
  i starts with 4 and goes to either 3 when extBits is greater or equal then 3, or something less than 3 otherwise. How is this loop supposed to work?
• DirStream has import org.apache.tinkerpop.gremlin.process.traversal.*
• FileSystemDaemon.getInstance is missing javadoc for isServerSideFS
• why do we need both filesystem and stream in OSResourceManager? Shouldn't this be an 'both or nothing'? Does this allow streams on server-side and file-system on client-side (or viceversa)?

Constantin Asofiei wrote:

Review for 4065a rev 14527:

• ConcurrentResource.resourceTypeName - the javadoc was indented right
• HighLevelObject.LOG - we are replacing java.util.logging in #5703, just something to keep track in #5703 or if #5703 gets merged before this
• LeafSessionManager.java, BitFlagsConstants.java, LogHelper.java - there is no change beside the end-of-file newline, this can be reverted
• ImportWorker has import com.goldencode.p2j.persist.pl.Functions - is this needed?
• BitFlagsResource - there is import edu.emory.mathcs.backport.java.util.* and import @edu.emory.mathcs.backport.java.util.Arrays;
• BitFlagsResource.isRightsSetValid - I don't understand this code:
  [...]
  i starts with 4 and goes to either 3 when extBits is greater or equal then 3, or something less than 3 otherwise. How is this loop supposed to work?
• DirStream has import org.apache.tinkerpop.gremlin.process.traversal.*
• FileSystemDaemon.getInstance is missing javadoc for isServerSideFS

I will address all the above.

• why do we need both filesystem and stream in OSResourceManager? Shouldn't this be an 'both or nothing'? Does this allow streams on server-side and file-system on client-side (or viceversa)?

Yes, currently you can configure the streams (currently file and dir streams) or file-system independently. I agree this could be merged together under a file-system resource. Greg, what do you think?

I agree this could be merged together under a file-system resource. Greg, what do you think?

Agreed.

I rebased 4065a against the latest trunk.

The points raised during the reviews are resolved in 4065a revision 14561. File history entries added in revision 14562. Please review.

Code Review Task branch 4065a Revisions 14560 through 14562

No objections. Weren't the logging changes removed on purpose to avoid conflicts with 5753d and 5703a?

Galya: Please review the logging changes in rev 14560.

Greg Shah wrote:

Galya: Please review the logging changes in rev 14560.

The changes are indeed unexpected.

Constantin Asofiei wrote:

Review for 4065a rev 14527:

• HighLevelObject.LOG - we are replacing java.util.logging in #5703, just something to keep track in #5703 or if #5703 gets merged before this

Also this is a surprising choice of logger: Logger.getLogger() - it doesn't use the standard LogHelper and will not cause compilation issue on rebase and might stay under the radar.

Greg Shah wrote:

Code Review Task branch 4065a Revisions 14560 through 14562

No objections. Weren't the logging changes removed on purpose to avoid conflicts with 5753d and 5703a?

I'm not aware of anybody removing them during trunk merge. At some point I manually added them to the branch and then removed. I assumed this has caused the removal during rebase.

Let's ensure that they don't cause conflicts with the upcoming merge of 5703a.

Also, we definitely should be using LogHelper.getLogger().

Hynek, I will obviously have to resolve the conflicts caused by all the changes you do to logging and then delete them. I hope that was clearly conveyed to you already. If you find it useful to have the changes on trunk for a few days, then I hope you enjoy them. :)

Greg Shah wrote:

Let's ensure that they don't cause conflicts with the upcoming merge of 5703a.

AFAIK Galya has successfully rebased with the logging changes. They are really minimal anyway and I don't think they should cause any troubles.

Also, we definitely should be using LogHelper.getLogger().

OK I will add it.

Hynek Cihlar wrote:

AFAIK Galya has successfully rebased with the logging changes. They are really minimal anyway and I don't think they should cause any troubles.

Sweet.

Galya Bogdanova wrote:

Hynek, I will obviously have to resolve the conflicts caused by all the changes you do to logging and then delete them. I hope that was clearly conveyed to you already. If you find it useful to have the changes on trunk for a few days, then I hope you enjoy them. :)

As long as the log format for the log entries stays consistent from the bootstrap on, then do whatever you like with my changes :-).

The change related to the reduction of package names seems to be useful.

Greg Shah wrote:

The change related to the reduction of package names seems to be useful.

Wasn't a requirement in #5703. Leave a note in #7279 for future Formatter changes.

Logger.getLogger replaced with LogHelper in 4065a revision 14563. Galya, please review.

Hynek Cihlar wrote:

Logger.getLogger replaced with LogHelper in 4065a revision 14563. Galya, please review.

There is nothing to review here.

I'm still not clear though if this is an acceptable working process - create an issue yourself, ask about permission to solve, don't receive it, understand it causes conflicts with an ongoing complete revamp of the same feature, then solve it in your own way in an unrelated task without notifying anyone about it explicitly and merge it to trunk, repeat last two twice? I mean can I do it as well, Greg? :)

Galya Bogdanova wrote:

Hynek Cihlar wrote:

Logger.getLogger replaced with LogHelper in 4065a revision 14563. Galya, please review.

There is nothing to review here.

I'm still not clear though if this is an acceptable working process - create an issue yourself, ask about permission to solve, don't receive it, understand it causes conflicts with an ongoing complete revamp of the same feature, then solve it in your own way in an unrelated task without notifying anyone about it explicitly and merge it to trunk, repeat last two twice? I mean can I do it as well, Greg? :)

If it's a tiny change fixing a bug and then the change is reviewed then I don't see any issue with that.

We could argue about the formatter change. But knowing the product and the customers for so long I was sure this would be a welcome change.

In any case I didn't want to sneak in any changes in. Everything was reviewed as was any other task branch.

The changes are so minimal that I don't see how it can cause issues during merging with your work. And I offered you help with any potential conflicts.

Hynek Cihlar wrote:

The changes are so minimal that I don't see how it can cause issues during merging with your work.

I'm not sure if you understand that the classes don't exist any more, so your changes are wiped out, if you don't request me to change my implementation in the last minute. That's why it's rude to pretend you don't hear, when you're warned someone does a refactoring on the same feature and don't care at all what it is. If you're so opinionated, so well familiar with the client and you don't want to discuss it, then I guess Greg had to give you the logging tasks. I'm completely fine to give you #5703 to take over it. Just ask the team leads to assign it to you.

Galya Bogdanova wrote:

Hynek Cihlar wrote:

The changes are so minimal that I don't see how it can cause issues during merging with your work.

I'm not sure if you understand that the classes don't exist any more, so your changes are wiped out, if you don't request me to change my implementation in the last minute.

The code doesn't matter. It's important so that we keep the product good. If it means you need to wipe out my changes, let's be it.

Hynek Cihlar wrote:

The code doesn't matter. It's important so that we keep the product good. If it means you need to wipe out my changes, let's be it.

Requirements go into related tasks, then get discussed and the best solution is taken. If you sneak changes in, they will get wiped out, not because it makes the product worse, but because of lack of proper communication and a fault in the work process created by your eagerness for improvement. I haven't seen an application code in a state of art, that doesn't need improvements, so you need to doubt your ideal solutions to make something perfect.

For example your vision of the product may have a flaw:
Hynek Cihlar wrote:

But knowing the product and the customers for so long I was sure this would be a welcome change.

I thought we're doing an open source framework, not a product for one specific customer.

It is not uncommon for us to implement small changes to unrelated areas in tasks that are otherwise addressing a different feature/purpose.

Since we do not use a traditional waterfall process, our design work tends to be done in stages during our tasks rather than all at once. This means we:

• Can start work faster and deliver in more incremental steps.
• Work more independently and asynchronously.
• Decentralize our design.
• Require more discussions, across more tasks.
• Can have changes later that were not foreseen at the beginning.
• Will have the work on specific features split over many tasks and many team members.

These last two items can be frustrating when you are working on a big update, but it is a consequence of our process. No one here is trying to make life harder for their teammates.

But knowing the product and the customers for so long I was sure this would be a welcome change.

I thought we're doing an open source framework, not a product for one specific customer.

We take feedback and requirements from our multiple existing customers into account when implementing changes that everyone will see. This is a good thing.

Hynek is correct that our customers have told us on many occasions that the stack traces are overwhelming. Reducing the text representation of the package names is a good enhancement, which is why we want to keep it.

If you're so opinionated, so well familiar with the client and you don't want to discuss it, then I guess Greg had to give you the logging tasks. I'm completely fine to give you #5703 to take over it. Just ask the team leads to assign it to you.

I would hope that we can integrate these changes together without moving all logging work to Hynek.

Greg Shah wrote:

I would hope that we can integrate these changes together without moving all logging work to Hynek.

Of course, when the requirements are defined in a task. I'll be waiting for it.

I think this discussion qualifies as defining the requirements for the "reduce package name text in stack trace output" feature. Hynek has written the changes and tested that they work. Rather than defer this to a separate work stream, let's discuss how his work can be used.

Please remove all the logging changes from rev 14563. Attach a patch for the package name abbreviation code to #7279. We will apply it to that task's branch when there is one. The other changes are not needed, as far as I know. This will eliminate any conflict with 5703a, which has 500+ files edited so we do want to make the 5703a rebase and merge process easier.

After 4065a is merged to trunk the following resources still need a server-side implementation:

• process launching
• XML
• web-services
• server sockets

Greg Shah wrote:

Please remove all the logging changes from rev 14563. Attach a patch for the package name abbreviation code to #7279.

OK, no problem. Just please note this will cause another round of code conflicts for Galya.

I reverted the log changes introduced in trunk revision 14535 in 4065a revision 14565.

Please merge 4065a to trunk.

4065a merged in to trunk revision 14554 and archived.

Constantin, what is currently the least stressful way to test web-services?

The wiki is here: Running Web Tests. You will need SoapUI, and possible need to edit the host and port.

XML load/save is already handled with the server-side file access. It is enough to activate server-side file resource.

Constantin Asofiei wrote:

The wiki is here: Running Web Tests. You will need SoapUI, and possible need to edit the host and port.

Converting Web tests gives the following errors. Any idea?

    [java] Null annotation (full-java-class) for oo.serialization.SerializeAll [CLASS_NAME] @1:47 (468151435277)
     [java] Null annotation (simple-java-class) for oo.serialization.SerializeAll [CLASS_NAME] @1:47 (468151435277)
     [java] Null annotation (containing-package) for oo.serialization.SerializeAll [CLASS_NAME] @1:47 (468151435277)
     [java] Null annotation (full-java-class) for define [DEFINE_PARAMETER] @1:1 (468151435267)
     [java] Null annotation (simple-java-class) for define [DEFINE_PARAMETER] @1:1 (468151435267)
     [java] Null annotation (containing-package) for define [DEFINE_PARAMETER] @1:1 (468151435267)
     [java] Null annotation (full-java-class) for oo.serialization.SerializeAll [CLASS_NAME] @2:46 (468151435291)
     [java] Null annotation (simple-java-class) for oo.serialization.SerializeAll [CLASS_NAME] @2:46 (468151435291)
     [java] Null annotation (containing-package) for oo.serialization.SerializeAll [CLASS_NAME] @2:46 (468151435291)
     [java] Null annotation (full-java-class) for define [DEFINE_PARAMETER] @2:1 (468151435281)
     [java] Null annotation (simple-java-class) for define [DEFINE_PARAMETER] @2:1 (468151435281)
     [java] Null annotation (containing-package) for define [DEFINE_PARAMETER] @2:1 (468151435281)
     [java] Null annotation (full-java-class) for serializedObject [VAR_CLASS] @5:1 (468151435298)
     [java] Null annotation (simple-java-class) for serializedObject [VAR_CLASS] @5:1 (468151435298)
     [java] Null annotation (containing-package) for serializedObject [VAR_CLASS] @5:1 (468151435298)
     [java] Null annotation (full-java-class) for objectToSerialize [VAR_CLASS] @5:20 (468151435300)
     [java] Null annotation (simple-java-class) for objectToSerialize [VAR_CLASS] @5:20 (468151435300)
     [java] Null annotation (containing-package) for objectToSerialize [VAR_CLASS] @5:20 (468151435300)
     [java] EXPRESSION EXECUTION ERROR:
     [java] ---------------------------
     [java] persist()
     [java] ^  { null value for annotation 'full-java-class':define [DEFINE_PARAMETER]:468151435267 @1:1
     [java]    input [KW_INPUT]:468151435269 @1:9
     [java]    objectToSerialize [SYMBOL]:468151435273 @1:26
     [java]    as [KW_AS]:468151435275 @1:44
     [java]       oo.serialization.SerializeAll [CLASS_NAME]:468151435277 @1:47
     [java]  }
     [java] ---------------------------
     [java] ERROR:
     [java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT      
     [java] -----------------------
     [java] Rule Type :   POST
     [java] Source AST:  [ block ] BLOCK/ @0:0 {468151435265}
     [java] Copy AST  :  [ block ] BLOCK/ @0:0 {468151435265}
     [java] Condition :  persist()
     [java] Loop      :  false
     [java] --- END RULE REPORT ---
     [java] 
     [java] 
     [java] 
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1099)
     [java]     at com.goldencode.p2j.convert.TransformDriver.processTrees(TransformDriver.java:587)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.back(ConversionDriver.java:563)
     [java]     at com.goldencode.p2j.convert.TransformDriver.executeJob(TransformDriver.java:999)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1284)
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:1
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:495)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:500)
     [java]     at com.goldencode.p2j.pattern.RuleContainer.apply(RuleContainer.java:590)
     [java]     at com.goldencode.p2j.pattern.RuleSet.apply(RuleSet.java:98)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1710)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1577)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1510)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1062)
     [java]     ... 4 more
     [java] Caused by: java.lang.NullPointerException: null value for annotation 'full-java-class':define [DEFINE_PARAMETER]:468151435267 @1:1
     [java]    input [KW_INPUT]:468151435269 @1:9
     [java]    objectToSerialize [SYMBOL]:468151435273 @1:26
     [java]    as [KW_AS]:468151435275 @1:44
     [java]       oo.serialization.SerializeAll [CLASS_NAME]:468151435277 @1:47
     [java] 
     [java]     at com.goldencode.ast.XmlFilePlugin.writeSingleAnnotation(XmlFilePlugin.java:1154)
     [java]     at com.goldencode.ast.XmlFilePlugin.writeAnnotations(XmlFilePlugin.java:1115)
     [java]     at com.goldencode.ast.XmlFilePlugin.writeAst(XmlFilePlugin.java:1074)
     [java]     at com.goldencode.ast.XmlFilePlugin.writeAst(XmlFilePlugin.java:1080)
     [java]     at com.goldencode.ast.XmlFilePlugin.writeAst(XmlFilePlugin.java:1080)
     [java]     at com.goldencode.ast.XmlFilePlugin.saveTree(XmlFilePlugin.java:518)
     [java]     at com.goldencode.ast.AstManager.saveTree(AstManager.java:360)
     [java]     at com.goldencode.p2j.pattern.CommonAstSupport$Library.persist(CommonAstSupport.java:2972)
     [java]     at com.goldencode.expr.CE10873.execute(Unknown Source)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:398)
     [java]     ... 11 more

Is oo/serialization/SerializeAll.cls included in the conversion list? All referenced OO classes/interfaces/enums in the entire object graph must be included in the conversion list.

Greg Shah wrote:

Is oo/serialization/SerializeAll.cls included in the conversion list? All referenced OO classes/interfaces/enums in the entire object graph must be included in the conversion list.

The class file is not in the conversion list, but is properly resolved and converted. Adding it to the conversion list indeed resolves the error. Does it really need to be in the list?

Yes, until #6082 is complete.

Greg, is the scope of the server-side web-services only the invocation part (i.e. WebServiceHelper, WebServiceImpl) or also the serving part (i.e. WebServiceHandler)?

It is the client side (calling) only.

I created task branch 4065b and checked in a fix for the regression reported in #5731-856.

ThinClient must be instantiated yet before the custom authentication hook is invoked.

Please review.

Hynek, I think there is some confusion: REST/SOAP/WebHandler are for the FWD web service support (server-side deployment). I think you mean the socket/server-socket, and SOAP clients - these are used by 4GL converted code, where currently the 4GL socket is being opened and managed on client-side.

I thought we already supported regular 4GL client sockets on the server side, but the missing part was the web services client calls.

The 4GL server sockets would be an additional item to move over.

Greg Shah wrote:

I thought we already supported regular 4GL client sockets on the server side,

Yes, 4GL client sockets are implemented.

but the missing part was the web services client calls.

Correct, web service client calls are implemented in p2j.util.WebServiceImpl.

The 4GL server sockets would be an additional item to move over.

Correct.

• for server-sockets, use the old testcases project, uast/sockets/socket-run.p - there are socket-client.p and socket-server.p programs which are ran by this.
• for web services (SOAP), Marian added tests in xfersrv01:testcases, but I haven't ran those - they rely on the FWD SOAP web service support (what I mentioned earlier), plus some 4GL testcases which connect to these services ran in FWD. There are also tests in uast/web_services, with a server/ folder, but I never ran this. Originally I used a 3rd-party web service URL, but that is down now - see uast/web_services/wsdl_test1.p.

I've found another free 3rd-party web service - see this simple test:

def var h as handle.
def var l as log.
def var hp as handle.
def var chi as char.
def var cho as char.

create server h.

l = h:connect("-WSDL http://webservices.oorsprong.org/websamples.countryinfo/CountryInfoService.wso?WSDL  -nohostverify  -Service CountryInfoService").
if l <> yes then do: message "could not connect". pause. quit. end.

run CountryInfoServiceSoapType set hp on server h.

if not valid-handle(hp) then do: message "could not run port". pause. quit. end.

chi = 'RO'.

output to log.txt.
run LanguageName in hp(input chi, output cho).

message cho.
output close.

Greg, also, something else to note: there is code in p2j.oo for client sockets and also HTTP connections. Marian, please remind me which p2j.oo classes are the implementation of the socket or HTTP call.

Constantin Asofiei wrote:

Greg, also, something else to note: there is code in p2j.oo for client sockets and also HTTP connections. Marian, please remind me which p2j.oo classes are the implementation of the socket or HTTP call.

The 'client socket' OO implementation is in oo.net.serverconnection.ClientSocket but the implementation is mostly only stub. The HTTP client uses the library from oo.net.http.lib.sockets. LegacySocketLibrary but that one is not using the 'client socket' as in 4GL but the apache http client.

Hynek Cihlar wrote:

I created task branch 4065b and checked in a fix for the regression reported in #5731-856.

ThinClient must be instantiated yet before the custom authentication hook is invoked.

Please review.

This addresses the regression mentioned in #5731-856.

Code Review Task Branch 4065b Revision 14571

No objections.

4065b revision 14576 implements server-side web service calling. Please review.

I named the "resource" just "webservice". It would probably make sense to rename it to webservice-client and distinguish it from the web service handler.

I checked in a fix for another client initialization problem in 4065b revision 14577. StreamDaemon initialization must be postponed after establishing user session. Please review.

Code Review Task Branch 4065b Revisions 14576 and 14577

1. Doesn't ClientSideResourceManager.initialize() need a new entry for WEBSERVICE_RESOURCE?

2. OSResourceManager.isServerSideWebService() should reference WEBSERVICE_RESOURCE instead of FILESYSTEM_RESOURCE.

3. Shouldn't we still call WebServiceImpl.initialize() (even though it is empty) so that if someone puts logic in there later, it will work?

Greg Shah wrote:

Code Review Task Branch 4065b Revisions 14576 and 14577

1. Doesn't ClientSideResourceManager.initialize() need a new entry for WEBSERVICE_RESOURCE?

2. OSResourceManager.isServerSideWebService() should reference WEBSERVICE_RESOURCE instead of FILESYSTEM_RESOURCE.

The above resolved im 4065b revision 14578.

3. Shouldn't we still call WebServiceImpl.initialize() (even though it is empty) so that if someone puts logic in there later, it will work?

The method is a SessionListener override and gets called by BaseSession.

I named the "resource" just "webservice". It would probably make sense to rename it to webservice-client and distinguish it from the web service handler.

Agreed. Perhaps the socket usage should also be noted as the socket-client version?

The method is a SessionListener override and gets called by BaseSession.

OK.

Are you working on process launching and server-sockets?

Greg Shah wrote:

Are you working on process launching and server-sockets?

No, I switched to my other priority tasks.

Are you working on process launching and server-sockets?

No, I switched to my other priority tasks.

Let's get this task complete first. I think when these two are added, we can close the task.

Greg, I'd like to merge 4065b to trunk to address the regressions.

Go ahead.

Greg Shah wrote:

Go ahead.

In progress.

4065b merged to trunk as revision 14580 and archived.

Hynek, there is a regression in trunk rev 14580 - this seems to fix it, I need to finish ETF testing to confirm.

=== modified file 'src/com/goldencode/p2j/ui/chui/ThinClient.java'
--- old/src/com/goldencode/p2j/ui/chui/ThinClient.java  2023-05-22 09:01:31 +0000
+++ new/src/com/goldencode/p2j/ui/chui/ThinClient.java  2023-05-24 16:07:55 +0000
@@ -3498,7 +3498,6 @@
       sigintThread = new SigintWaiter(this);

       new ComOleDaemon(single);
-      pd   = new ProcessDaemon(tk.getInstanceDriver().getChildProcessFactory(), single, sd);
       ospd = new OsPropertiesDaemon(single);

       // load the FWD native libraries
@@ -3677,6 +3676,7 @@
    {
       fileSystem = OSResourceManager.getInstance().initializeFileSystem();
       sd = new StreamDaemon(this, single);
+      pd = new ProcessDaemon(tk.getInstanceDriver().getChildProcessFactory(), single, sd);
    }

    /**

Constantin Asofiei wrote:

Hynek, there is a regression in trunk rev 14580 - this seems to fix it, I need to finish ETF testing to confirm.

This patch fixes the problem in my testing.

Constantin Asofiei wrote:

Constantin Asofiei wrote:

Hynek, there is a regression in trunk rev 14580 - this seems to fix it, I need to finish ETF testing to confirm.

This patch fixes the problem in my testing.

Good, it looks OK to me.

I implemented server sockets to be a distinct resource from client sockets. So currently one can define server sockets to be server-side while client-sockets to execute on the client. Now server side sockets use the client sockets for the accepted connections, and so the configuration may lead to listening socket running on the server but connection socket on the client. This is confusing and I can't think of a use case where this would be desired. Thus I will move the server and client sockets under single resource "socket".

For the socket client the key store password is read from the client parameters ssl-socket:truststore:password. However for the listening socket the password is read from ssl-socket:keystore:password. This doesn't seem right. Should the password be read from the same location for both?

Hynek Cihlar wrote:

For the socket client the key store password is read from the client parameters ssl-socket:truststore:password. However for the listening socket the password is read from ssl-socket:keystore:password. This doesn't seem right. Should the password be read from the same location for both?

OK, according to Certificates_and_Keys_for_4GL_Language_Features it is correct.

Greg, for the process resource, launch will be conducted on the server only when no terminal is needed. That is for the OS-COMMAND with NO-CONSOLE and NO-WAIT, and for INPUT/OUTPUT/INPUT-OUTPUT THROUGH with file-based streams. For the other cases launch will be passed to the client as usual.

I implemented server sockets to be a distinct resource from client sockets. So currently one can define server sockets to be server-side while client-sockets to execute on the client.

This is optimal. It is possible that either one might need to be split out to the actual client while the other can be safely used on the server.

Now server side sockets use the client sockets for the accepted connections, and so the configuration may lead to listening socket running on the server but connection socket on the client. This is confusing and I can't think of a use case where this would be desired.

I agree this is not useful.

Thus I will move the server and client sockets under single resource "socket".

What is the effort to decouple these so that the server's inbound connections do not use the client socket resources?

Greg, for the process resource, launch will be conducted on the server only when no terminal is needed. That is for the OS-COMMAND with NO-CONSOLE and NO-WAIT, and for INPUT/OUTPUT/INPUT-OUTPUT THROUGH with file-based streams. For the other cases launch will be passed to the client as usual.

Understood. For the terminal cases, even though we could use ncurses (via libp2j.so in the server), we would have to move some substantial part of the UI processing to the server. Another reason is that our native terminal processing is probably not necessarily thread safe. And I'm unsure if the pseudoterminal support in Linux has any limitations on threading, we would have to investigate that aspect.

Greg Shah wrote:

What is the effort to decouple these so that the server's inbound connections do not use the client socket resources?

This shouldn't take more than 1 MD.

Hynek Cihlar wrote:

Greg Shah wrote:

What is the effort to decouple these so that the server's inbound connections do not use the client socket resources?

This shouldn't take more than 1 MD.

Please go ahead with it so that we can keep two independent resources (socket-server and socket-client).

Greg Shah wrote:

Hynek Cihlar wrote:

Greg Shah wrote:

What is the effort to decouple these so that the server's inbound connections do not use the client socket resources?

This shouldn't take more than 1 MD.

Please go ahead with it so that we can keep two independent resources (socket-server and socket-client).

OK

Is the fix for #4065-150 in a branch?

Greg Shah wrote:

Is the fix for #4065-150 in a branch?

Not yet AFAIK. I will create task branch 4065d for this. 4065c already contains changes for the listening socket and process.

The regression fix in #4065-150 was checked in 4065d and is ready to be merged to trunk once it passes review.

Code Review Task Branch 4065d Revision 14593

No objections. You can merge to trunk now.

4065d rebased and merged to trunk as revision 14594. The branch was archived.

This feature is not properly documented yet. As noted in #6075-25 and #4065-18, it can configured with a server-side-resources node like this:

                <node class="string" name="server-side-resources">
                  <node-attribute name="value" value="memptr,library"/>
                </node>

The cfg reading can be seen in the constructor for OSResourceManager. The values that can be used in the comma-separated string are:

• memptr
• library
• socket
• filesystem
• webservice

There is also the special all value which enables server-side processing for all supported resources. At this time, server-side logging is configured and implemented separately.

As with all of our context-specific 4GL runtime options, this can be set at global default, server default, server, group/account levels. A simple test with a global setting would place this in /server/default/runtime/default/.

This configuration is not enough. For example, to enable server-side filesystem, you also need:

<node class="strings" name="resource-plugins">
  <node-attribute name="values" value="com.goldencode.p2j.security.FileSystemResource"/>
</node>

And this on its turn is still not enough, because read and write permissions are still missing for the user. I can't remember how it should be configured.

Galya B wrote:

This configuration is not enough. For example, to enable server-side filesystem, you also need:

[...]

And this on its turn is still not enough, because read and write permissions are still missing for the user. I can't remember how it should be configured.

You typically configure the ACLs in FWD Admin.

Here is an example of a file-system ACL entry:

        <node class="container" name="file-system">
          <node class="container" name="005500">
            <node class="resource" name="resource-instance">
              <node-attribute name="reference" value="ls"/>
              <node-attribute name="reftype" value="TRUE"/>
            </node>
            <node class="fileSystemRights" name="rights">
              <node-attribute name="permissions" value="'00010000'B"/>
            </node>
            <node class="strings" name="subjects">
              <node-attribute name="values" value="all_others"/>
            </node>
          </node>
       </node>

Where the reference points to a file system object (ls command in this case) and the permissions attribute contains the assigned permissions. For their values look in FileSystemRights and BitFlagsConstants Java classes.

Greg Shah wrote:

Please complete this work for all client APIs other than UI.

What has to be completed?

How do client systems work on a server process started without admin permissions?

Galya B wrote:

Greg Shah wrote:

Please complete this work for all client APIs other than UI.

What has to be completed?

Look for references to RemoteObject.obtainInstance and obtainNetworkInstance - this is referenced in classes like ProcessOps, EnvironmentOps, SmtpEmail, Environments, SessionUtils. RemoteObject is what it requires a connection to the FWD client.

How do client systems work on a server process started without admin permissions?

The OS access will be done using the FWD server's account. What do you mean by 'admin permissions'?

Constantin Asofiei wrote:

The OS access will be done using the FWD server's account. What do you mean by 'admin permissions'?

As far as I know the server process runs in the OS without OS admin permissions, while the clients can run under an OS admin user. How is for example the file system going to be accessed the same way server-side, when the server process is not running as admin?

That is a problem for the customer to resolve. If their application allows running multiple users using the same account then that will work.

For example, customers that use PASOE work this way already in OpenEdge. In this case, the customer must run the FWD server JVM in the same account as they run PASOE. If the application works in OE then it will work for us.

You can run the FWD server process in an OS admin account but it is a terrible idea. Nevertheless, some customers might be doing this in PASOE and if so, they'll have to do the same in FWD.

Either way, it is not something we handle.

Is 4065c merged?

Greg Shah wrote:

Please complete this work for all client APIs other than UI.

Do we need the UI for the #8661? Appservers write the output (outputToFile) to /dev/null.

Galya B wrote:

Greg Shah wrote:

Please complete this work for all client APIs other than UI.

Do we need the UI for the #8661? Appservers write the output (outputToFile) to /dev/null.

Maybe, but that will be done separate from #4065.

Is 4065c merged?

I don't know the status of it, but since there is no discussion here it is unlikely it was merged unless the contents were included in 4065d.

Greg Shah wrote:

Galya B wrote:

Greg Shah wrote:

Please complete this work for all client APIs other than UI.

Do we need the UI for the #8661? Appservers write the output (outputToFile) to /dev/null.

Maybe, but that will be done separate from #4065.

My point here is if the UI change is needed for #8661 we need the separate task assigned now.

My point here is if the UI change is needed for #8661 we need the separate task assigned now.

Go ahead and define a task (in the UI project). I don't know that I have someone to assign to it yet. I think it can be done in parallel to but slightly behind both #8661 and #4065.

The environment resource depends on the filesystem. It's not a valid case to have client-side filesystem and server-side environment, but at the same time server-side filesystem can't be enabled implicitly, because it requires security configs.

For SmtpEmail can I successfully send a message locally using the example in Email_Send? Is the code just using an existing mail server on the specified host or sets a server itself?

Galya B wrote:

For SmtpEmail can I successfully send a message locally using the example in Email_Send? Is the code just using an existing mail server on the specified host or sets a server itself?

OK, a more important question: where is the send button in the form :)

SessionUtils uses SessionExports to only push date/time/format configs to the client, while writing the same server-side to SessionDateUtils, so it simply needs a config to be added to the conditional instantiation of SessionExports. This config probably will have to be only used for headless clients. I'm not clear how com.goldencode.p2j.util.date is used client-side. Is it something to be migrated server-side?

ProcessOps launches a new child process remotely (from the client JVM). Such functionality is not a good idea to go on the server. It can launch a process / many processes exhausting resources, incapacitating the server.

The environment resource depends on the filesystem. It's not a valid case to have client-side filesystem and server-side environment, but at the same time server-side filesystem can't be enabled implicitly, because it requires security configs.

The customer is responsible for ensuring that:

• Any code that accesses the filesystem is safe for use from the single server process. This may require 4GL code changes but in an application already running in PASOE, this is already known to be OK (since it is how it works in OE).
• Any ACLs that are needed are created.

In PASOE, they don't provide any mechanism to secure the filesystem access. We do, but in this case we could introduce an option in the PASOE mode enablement to implicitly create an ACL to open access up for PASOE sessions.

For SmtpEmail can I successfully send a message locally using the example in Email_Send? Is the code just using an existing mail server on the specified host or sets a server itself?

OK, a more important question: where is the send button in the form :)

In ChUI, they often don't have buttons for "saving" the results of editing a form. Instead, it is a common practice to use the F1 key (a.k.a. the high level "GO" event).

Is the code just using an existing mail server on the specified host or sets a server itself?

Yes, there must be an external SMTP server.

Greg Shah wrote:

For SmtpEmail can I successfully send a message locally using the example in Email_Send? Is the code just using an existing mail server on the specified host or sets a server itself?

OK, a more important question: where is the send button in the form :)

In ChUI, they often don't have buttons for "saving" the results of editing a form. Instead, it is a common practice to use the F1 key (a.k.a. the high level "GO" event).

On F1 it says "No application help is available", but it doesn't send the form.

Galya B wrote:

SessionUtils uses SessionExports to only push date/time/format configs to the client, while writing the same server-side to SessionDateUtils, so it simply needs a config to be added to the conditional instantiation of SessionExports. This config probably will have to be only used for headless clients. I'm not clear how com.goldencode.p2j.util.date is used client-side. Is it something to be migrated server-side?

No, the date class is heavily used on both sides. Nothing is needed there other than to bypass any client synching.

For SmtpEmail can I successfully send a message locally using the example in Email_Send? Is the code just using an existing mail server on the specified host or sets a server itself?

OK, a more important question: where is the send button in the form :)

In ChUI, they often don't have buttons for "saving" the results of editing a form. Instead, it is a common practice to use the F1 key (a.k.a. the high level "GO" event).

On F1 it says "No application help is available", but it doesn't send the form.

The F2 key is the HELP function/event. Why would you think that invoking help will save the form?

ProcessOps launches a new child process remotely (from the client JVM). Such functionality is not a good idea to go on the server. It can launch a process / many processes exhausting resources, incapacitating the server.

Yes, child processes are messy, fragile, resource intensive and a huge security hole.

We still have to support them on the server side.

Greg Shah wrote:

On F1 it says "No application help is available", but it doesn't send the form.

The F2 key is the HELP function/event. Why would you think that invoking help will save the form?

I mistyped. I'm testing F1 live.

Galya B wrote:

Greg Shah wrote:

On F1 it says "No application help is available", but it doesn't send the form.

The F2 key is the HELP function/event. Why would you think that invoking help will save the form?

I mistyped. I'm testing F1 live.

There is a client issue:

24/08/15 16:30:51.295+0300 | WARNING | com.goldencode.p2j.ui.client.event.EventManager | PID:83824, ThreadName:main, Session:00000066, User:bogus | The unicode codepoint '301' cannot be represented by this codeset 'ISO-8859-1'

On F1 it says "No application help is available", but it doesn't send the form.

The F2 key is the HELP function/event. Why would you think that invoking help will save the form?

I mistyped. I'm testing F1 live.

There is a client issue:

24/08/15 16:30:51.295+0300 | WARNING | com.goldencode.p2j.ui.client.event.EventManager | PID:83824, ThreadName:main, Session:00000066, User:bogus | The unicode codepoint '301' cannot be represented by this codeset 'ISO-8859-1'

This looks like a regression.

Greg Shah wrote:

The environment resource depends on the filesystem. It's not a valid case to have client-side filesystem and server-side environment, but at the same time server-side filesystem can't be enabled implicitly, because it requires security configs.

The customer is responsible for ensuring that:

• Any code that accesses the filesystem is safe for use from the single server process. This may require 4GL code changes but in an application already running in PASOE, this is already known to be OK (since it is how it works in OE).
• Any ACLs that are needed are created.

In PASOE, they don't provide any mechanism to secure the filesystem access. We do, but in this case we could introduce an option in the PASOE mode enablement to implicitly create an ACL to open access up for PASOE sessions.

My point is that if only the resource "environment" is enabled as server-side, which can be done for any client, not only PASOE, it internally has a dependency on the "filesystem" resource that is not configured server-side. I think it should be considered a misconfiguration leading to a fail.

As for PASOE it's not a matter of choice to enable or not certain resources server-side, all should be enabled, so we can do more implicit work, as you suggested.

The environments depend on ClientStorage. ClientStorage is used for the enhanced browser configs, the registry fallback in Linux, the selected theme. ClientStorage can be the JVM user preferences OR the web page local storage (with webClient/useLocalStorage).

I think this is what should happen: When environments is enabled server-side, then it creates its server-side instance of LocalStorage writing to the JVM storage, that is to be used only by environments. Which means the same client will write the environment files / vars server-side and any other data client-side (in the user preferences or localStorage).

I think this is what should happen: When environments is enabled server-side, then it creates its server-side instance of LocalStorage writing to the JVM storage, that is to be used only by environments.

This seems reasonable. With "only by environments", you mean anything read/written by EnvironmentOps?

Which means the same client will write the environment files / vars server-side and any other data client-side (in the user preferences or localStorage).

You mean the enhanced browser stuff and the selected theme?

Greg Shah wrote:

I think this is what should happen: When environments is enabled server-side, then it creates its server-side instance of LocalStorage writing to the JVM storage, that is to be used only by environments.

This seems reasonable. With "only by environments", you mean anything read/written by EnvironmentOps?

EnvironmentDaemon. I haven't started on EnvironmentOps.

Which means the same client will write the environment files / vars server-side and any other data client-side (in the user preferences or localStorage).

You mean the enhanced browser stuff and the selected theme?

Yes.

EnvironmentOps is forwarding certain methods to EnvironmentDaemon, this is what's common.

Why is new MemoryDaemon(single) first called with false on ServerSideResourceManager.initializeMemptrResource and then with true (when com.goldencode.p2j.util.memptr#ctxt is accessed) on the server, when server-side resource memptr is enabled? The same with new LibraryDaemon(single) and NativeInvoker, when library is enabled server-side.

In current implementation, you can have i.e. memptr on server-side and native APIs on client-side. But, native APIs can use memptr, also - so they need a network server registered, to call back into the FWD server, to use the memptr. This is why there is both a local server and a remote server registered for memptr, native APIs and socket.

If you look in ClientSideResourceManager (used by FWD client), there is this code:

   public LowLevelBuffer initializeMemptr()
   {
      LowLevelBuffer buf = (LowLevelBuffer) resources.get(MEMPTR_RESOURCE);
      if (buf != null)
      {
         return buf;
      }

      if (isServerSideMemptr())
      {
         buf = (LowLevelBuffer) RemoteObject.obtainNetworkInstance(LowLevelBuffer.class);
      }
      else
      {
         buf = new MemoryDaemon(false);
      }

      resources.put(MEMPTR_RESOURCE, buf);

      return buf;
   }

which ensures that when the memptr is on server-side, FWD client uses the LowLevelBuffer on the FWD server, and not a direct MemoryDaemon instance.

I guess the default approach with migrating to server-side resources is to rework client dependencies and not register the interface as a network server. The problem with registering one instance of the environments interface server-side is that is has a dependency on session attributes. So I wonder which one to try and rework: the client dependencies or the interface/instance to make it session independent.

I'm somewhat confused, will appreciate any hint. To allow for a complete server-side execution we should not be leaving any client calls to the server and they need to be reworked. So these registered servers will have to be removed. Under what configuration is it supposed to be implemented? UI? I have a FontManager asking for the environment type client-side. Do I need to register a server server-side to allow the call or change the flow? I would prefer to clean up the client dependency in this case. But under what config will the calls to memptr from the client be reworked?

To allow for a complete server-side execution we should not be leaving any client calls to the server and they need to be reworked. So these registered servers will have to be removed.

The FontManager use case only happens in GUI, which we currently will not be implementing on the server side. So, we just need that code to keep working, but it doesn't need to be reworked otherwise.

In other words, non-UI server-side execution does need rework to ensure it is all there. For the server-side UI, it will only be the minimum ChUI needed in #9055 and that will not include GUI.

Do I need to register a server server-side to allow the call or change the flow? I would prefer to clean up the client dependency in this case.

I don't think we can remove the client dependency. Even when we have some sessions completely server-side, the client sessions may have no server-side support enabled (or just partial server-side support enabled).

But under what config will the calls to memptr from the client be reworked?

This one is different in my mind because it is often tightly coupled with the library calls usage.

I would prefer to force that wherever library calls were being executed, that is where the memptr support should also be executed. So, they can both be left on the client or both be moved to the server, but we don't split them between the two.

Constantin: Is there a customer requirement that means we can't do that?

Greg Shah wrote:

To allow for a complete server-side execution we should not be leaving any client calls to the server and they need to be reworked. So these registered servers will have to be removed.

The FontManager use case only happens in GUI, which we currently will not be implementing on the server side. So, we just need that code to keep working, but it doesn't need to be reworked otherwise.

In other words, non-UI server-side execution does need rework to ensure it is all there. For the server-side UI, it will only be the minimum ChUI needed in #9055 and that will not include GUI.

This FontManager.init is called by ThinClient.initializePost by all drivers. So if only 'environments' are enabled server-side, even with a batch client, the call is still made.

Do I need to register a server server-side to allow the call or change the flow? I would prefer to clean up the client dependency in this case.

I don't think we can remove the client dependency. Even when we have some sessions completely server-side, the client sessions may have no server-side support enabled (or just partial server-side support enabled).

This particular occurrence I can easily rework, but I have no idea how many such dependencies are out there and what will be broken next, that's why the discussion. From what I'm receiving, the server should not register a network interface for a server-side resource to be called by the client. It seems right, so I'll fix this one and check if any other such places in the code are run by non-UI code, assuming most are UI related.

Actually the call is for gui in FontManager.initialize. So I'll ignore it and test with chui.

Galya B wrote:

Actually the call is for gui in FontManager.initialize. So I'll ignore it and test with chui.

But then it should be a known limitation, the server-side resource 'environments' can't work with gui.

Greg Shah wrote:

BTW, I think we would be better off removing the process:arch:single=true mode. We never use it and it just complicates the code. We don't have to do it here, but then again we'll be editing the same locations anyway.

I'll do it in 4065e, because it complicates reasoning about the changes.

The client registers certain servers ClientExports, SessionExports, LegacyLogManagerClientService, etc. Under what config should these be transferred server-side?

• SessionExports is purely setting date props client-side in SessionDateUtils.
• LegacyLogManagerClientService basically writes LOG-MANAGER logs client-side, this should be dependent on the filesystem.
• ClientExports is anything and everything.

Shouldn't opsys (OPSYS function), when picked up from the client, be related to the filesystem?

Also is the default SESSION:PRINTER-NAME related to the file system or the env config?

Both are placed in an interface that doesn't make sense, so I'm moving them. Just don't know where to yet.

Galya B wrote:

Shouldn't opsys (OPSYS function), when picked up from the client, be related to the filesystem?

Also is the default SESSION:PRINTER-NAME related to the file system or the env config?

Both are placed in an interface that doesn't make sense, so I'm moving them. Just don't know where to yet.

If the answers are no, I'll move them to ClientExports.

ClientExports and ServerExports is both UI-specific. We should avoid these in clientless mode and otherwise leave them alone.

Galya B wrote:

Shouldn't opsys (OPSYS function), when picked up from the client, be related to the filesystem?

Also is the default SESSION:PRINTER-NAME related to the file system or the env config?

Both are placed in an interface that doesn't make sense, so I'm moving them. Just don't know where to yet.

These are most related to the environment of the existing interfaces.

4065e r15386 based on trunk r15381 for review.

• Added support for EnvironmentAccessor server-side under the new resource type 'environments'. Wrapper class Environments removed.
• OSResourceManager and concrete classes, resource interfaces and impl classes moved to the new package com.goldencode.p2j.util.osresource, which allows initialization of resource classes only in the package. initialize* methods made package-private and public static getters added to OSResourceManager for convenience.
• ServerSideResourceManager to cache the remote objects instead of creating new ones with every call, similarly to ClientSideResourceManager.
• OsPropertiesDaemon and OperatingSystemInspector removed, methods moved to EnvironmentDaemon and FileSystemDaemon.
• Single client removed and the method arguments called 'single' renamed + javadoc changed.

Hynek: Please review.

I've started testing. NPE fixed in r15387.

Server-side 'environments' auto enables server-side 'filesystem', but com.goldencode.p2j.security.FileSystemResource should be added to resource-plugins in directory, because there is a client-side check for the mandatory plugin.

Code review 4065e.

Overall pretty nice set of changes!

OsResourceType.java, wrong date in the file history entry.

import javax.print.* in ThinClient is redundant.

In WindowManager the assignment

      private boolean isChUI = ThinClient.getInstance() == null || ThinClient.getInstance().isChui();

StreamFactory.java wrong copyright year.

ClientSideResourceManager.initializeEnvironments javadoc wording is not right.

For EnvironmentAccessor.subscribe how is the serialization of consumer resolved in case environment is server-side?

Hynek Cihlar wrote:

In WindowManager the assignment
[...]
What is the connection between ThinClient.getInstance() null and chui mode? Is it possible ThinClient.getInstance() null in GUI mode?

With server-side 'environments' EnvironmentDaemon.unload calls WindowManager, which initializes the WorkArea, which is originally designed only for client-side use (depending on existing ThinClient instance), so it ends up in NPE. My idea with the change is that if there is no client, gui is not supported.

StreamFactory.java wrong copyright year.

It is Copyright (c) 2006-2024, Golden Code Development Corporation.. Do you mean a different file?

Galya B wrote:

Hynek Cihlar wrote:

In WindowManager the assignment
[...]
What is the connection between ThinClient.getInstance() null and chui mode? Is it possible ThinClient.getInstance() null in GUI mode?

With server-side 'environments' EnvironmentDaemon.unload calls WindowManager, which initializes the WorkArea, which is originally designed only for client-side use (depending on existing ThinClient instance), so it ends up in NPE. My idea with the change is that if there is no client, gui is not supported.

Can you prevent WindowManager to be called in this case? I'm worried about the case when on the client ThinClient.getInstance() will unexpectedly return null and this will be turned into ChUI mode.

StreamFactory.java wrong copyright year.

It is Copyright (c) 2006-2024, Golden Code Development Corporation.. Do you mean a different file?

Yes, I meant StreamFactory.java, so please ignore this.

Hynek Cihlar wrote:

With server-side 'environments' EnvironmentDaemon.unload calls WindowManager, which initializes the WorkArea, which is originally designed only for client-side use (depending on existing ThinClient instance), so it ends up in NPE. My idea with the change is that if there is no client, gui is not supported.

Can you prevent WindowManager to be called in this case? I'm worried about the case when on the client ThinClient.getInstance() will unexpectedly return null and this will be turned into ChUI mode.

If it returns null, the client crashes. I was thinking of avoiding the call to WindowManager, but I don't see requirements that server-side 'environments' will run only in a mode that doesn't rely on WindowManager. I would imagine it's still needed.

Editted

Galya B wrote:

Hynek Cihlar wrote:

With server-side 'environments' EnvironmentDaemon.unload calls WindowManager, which initializes the WorkArea, which is originally designed only for client-side use (depending on existing ThinClient instance), so it ends up in NPE. My idea with the change is that if there is no client, gui is not supported.

Can you prevent WindowManager to be called in this case? I'm worried about the case when on the client ThinClient.getInstance() will unexpectedly return null and this will be turned into ChUI mode.

If it returns null, the client crashes.

Exactly, it would fail fast.

I was thinking of avoiding the call to WindowManager, but I don't see requirements that server-side 'environments' will run only in a mode that doesn't rely on WindowManager. I would imagine it's still needed.

Fair enough. But please add a code comment explaining this case, as it is now it isn't obvious.

For EnvironmentAccessor.subscribe how is the serialization of consumer resolved in case environment is server-side?

This is a good question. The subscribe mechanism in EnvironmentDaemon is currently used only to listen to changes in the client/lang property in web clients, but theoretically it can be used for other properties as well. The listener should be called on the client. I see two solutions:
1. remove the env prop subscription mechanism altogether and call a new ClientExports method from EnvironmentOps.setCurrentLanguage that will call the web driver's setCurrentLanguage method;
2. create two new rpc methods in some relevant remote objects (with server-side environments there is no new registered network server on both sides: one method to request subscription for a prop on the server) and one to be called on the client, where the listeners will be saved.

If it returns null, the client crashes.

Whenever possible, we don't want a crash. For a critical problem, it would be better to generate a SEVERE log entry and fallback to a safe default rather than NPE.

Galya B wrote:

1. remove the env prop subscription mechanism altogether and call a new ClientExports method from EnvironmentOps.setCurrentLanguage that will call the web driver's setCurrentLanguage method;

This one seems to be the cleaner one.

All requested changes are in r15389.

4065e tested for regressions with hotel_gui and a customer project. Tested server-side and client-side environments and a few affected OE attrs in testcases. Tested the new set language approach. Should be ready for merge.

4065f r15407 fixing NPE in conversion ready for review.

Hynek, are you able to conduct the review for 4065f? I am not familiar with the changes.

Alexandru Lungu wrote:

Hynek, are you able to conduct the review for 4065f? I am not familiar with the changes.

I'm on it.

Code review 4065f. The changes look good.

The change has been tested with conversion of hotel_gui.

When do I merge?

Galya B wrote:

The change has been tested with conversion of hotel_gui.

When do I merge?

You can merge now.

Is the SMTP mail sender FWD-SMTP-EMAIL used by any customer?

I think it's not working because TLS1 and TLS1.1 are not supported any more (disabled in newer javas in jdk.tls.disabledAlgorithms and not supported by mail servers like outlook). In this case it throws javax.net.ssl.SSLHandshakeException: No appropriate protocol (protocol is disabled or cipher suites are inappropriate). It will require update to libraries and probably the code to make it work with TLS1.2.

I will move the EmailDaemon to server-side with a new resource config 'mail', but it won't be tested completely.

Is the SMTP mail sender FWD-SMTP-EMAIL used by any customer?

Not in production. Use of it in production is not expected soon.

I will move the EmailDaemon to server-side with a new resource config 'mail', but it won't be tested completely.

OK

There seem to be some loopholes in the server-side filesystem and its dependency on streams. For example:

Progress.Json.Objectmodel.JsonConstruct#writeStream(character) uses StreamFactory#findOutputStream(character), that looks up the stream name with StreamWrapper.findRemoteStreamId (streamByName map is filled in by streams of type RemoteStream). If the stream is a FileStream with server-side filesystem (instead of RemoteStream) the name will not be found in the map, and the method will throw error could not find open stream. This hasn't been tested, the conclusion is based purely on the code.

RemoteStream uses the network interface LowLevelStream. RemoteStream is not to be used with server-side resources, so it seems LowLevelStream doesn't need to be migrated, but it's part of StreamDaemon, that is also a StreamBuilder that is to be allowed server-side. I'll probably try to split them.

StreamDaemon is not a single resource, but a common wrapper for multiple resources, so I think its network interface will need to be split into other classes. For example openFileStream methods should go into FileSystemDaemon. The problem is that there are a lot of entangled 'resources', I'm drawing a spaghetti graph on paper to keep track of it all.

I'm still trying to figure out if there is an issue with the map of stream ids that is kept currently client-side in StreamDaemon, and if there are any calls on the server that depend on those ids with server-side resources.

StreamDaemon has a map of stream ids and the methods streamFromId, idFromStream, store (the last assigns ids to new streams). The methods for retrieving the id / stream are only used in ThinClient, quite excessively, in methods both used from classes on the client and exposed to the server via ClientExports.

Currently the only streams that can be enabled server-side are FileStream and DirStream, and they don't have ids associated with them, because the remote call to StreamDaemon (where store is executed) is not used. And since RemoteStream is also not used, the lookup by names can't be used too (as explained in the previous note).

To assign ids server-side the generation of ids can be synced between the client and the server (when only some of the streams are server-side), the streams will obviously be only recorded in maps in the respective process, but there are two problems related to the abstraction of the streams. For example the execution of a program requires to access server-side a stream by id:
1. if the id is not found in the map server-side, since the type of the stream is unknown, there should always be a check to the client (if such exists);
2. there could be possible calls to streamFromId, idFromStream client-side, when a stream type is enabled server-side and in this case the id will not be found. At the same time the two methods can't be simply exported as rpc for the obvious reason of serializing the stream.

Overall it gets too overcomplicated: the attempt to separate resources and allow only some to be enabled server-side, while others are client-side.

'environments' depends on 'filesystem', 'filesystem' depends on stream ids. The streams can be of type file, dir, process, web, clipboard, terminal, printer, but all of them share the same mechanism of being retrieved by id. I can't distinguish between the ways they are used and if some don't depend on the ids, because all of them are simply Stream.

So what I'm saying is that there doesn't seem to be a reasonable solution that will allow these resources to live split between the server and the client.

In regard to co-dependencies between environments and filesystem:

1. Best case is to separate these dependencies (make them independent).
2. If not reasonably possible, then we can link the two (when one is server side, they both are).

In regard to the steams and in particular about ids and usage of RemoteStream, I would prefer to leave the structure as intact as possible rather than huge refactoring. In other words, why not use our RemoteStream even on the server side? If the LowLevelStream instance is local, why is that a problem?

I don't understand the point about syncing the ids server and client. I would expect that for a given session, all ids will be generated wherever the LowLevelStream instance is implemented.

Greg Shah wrote:

In regard to co-dependencies between environments and filesystem:

1. Best case is to separate these dependencies (make them independent).

The .ini files depend on the filesystem impl. It is troublesome to split it.

1. If not reasonably possible, then we can link the two (when one is server side, they both are).

This is currently on trunk.

In regard to the steams and in particular about ids and usage of RemoteStream, I would prefer to leave the structure as intact as possible rather than huge refactoring. In other words, why not use our RemoteStream even on the server side? If the LowLevelStream instance is local, why is that a problem?

I don't understand the point about syncing the ids server and client. I would expect that for a given session, all ids will be generated wherever the LowLevelStream instance is implemented.

Let's make this clear: file, dir, process, web, clipboard, terminal, printer streams - all will be either server-side, or client-side with one config?

Let's make this clear: file, dir, process, web, clipboard, terminal, printer streams - all will be either server-side, or client-side with one config?

I think file, dir, web, terminal, printer are all reasonably done with in the same location.

clipboard is only available in GUI and does not get support on the server side.

The only tricky part here is process, because that should be in the same place as shell command execution. Again, if we need to link shell and filesystem, I'm OK with that.

Greg Shah wrote:

clipboard is only available in GUI and does not get support on the server side.

Clipboard is a widget, but ClipboardStream (Implements the output stream to write data into the client's system clipboard.) is still a stream, which means it will be used on the server and the id needs to be used by the server and by the client (as far as I understand how the ids are used). The ids should be unique, so the ids and the references to the streams can be stored either on the client or on the server. The other option of serializing streams and syncing ids between the client and the server doesn't seem a good one. Am I missing something?

Both the CLIPBOARD system handle (not actually a widget, just a kind of weird 4GL "object") and the ClipboardStream will only be used from GUI code. We can raise a legacy error in the scenario that a session tries to access this while server-side filesystem is turned on.

I need an example in OE that uses ClipboardStream, when converted.

Galya B wrote:

I need an example in OE that uses ClipboardStream, when converted.

This should do:

DEFINE VARIABLE counter AS INT NO-UNDO.
counter = 0.

OUTPUT TO "CLIPBOARD".

DO WHILE counter < 1000:
  counter = counter + 1.
  DISPLAY "TEXT".
END.

OUTPUT CLOSE.

Greg Shah wrote:

The only tricky part here is process, because that should be in the same place as shell command execution. Again, if we need to link shell and filesystem, I'm OK with that.

We'll be linking filesystem, all streams and environments. As for the clipboard stream, I still don't see how it's different from the other streams based on the code.

RemoteStream doesn't make a difference between the streams, when calling LowLevelStream, so there is no way to have part of them server-side, part client-side.

Galya B wrote:

Greg Shah wrote:

The only tricky part here is process, because that should be in the same place as shell command execution. Again, if we need to link shell and filesystem, I'm OK with that.

We'll be linking filesystem, all streams and environments. As for the clipboard stream, I still don't see how it's different from the other streams based on the code.

It is more a factor of where it could be used, which is only in GUI.

Galya B wrote:

RemoteStream doesn't make a difference between the streams, when calling LowLevelStream, so there is no way to have part of them server-side, part client-side.

I'm not suggesting RemoteStream would be used in the client side. I'm saying that we should retain the concept that the server side is accessing streams using a RemoteStream (inside a StreamWrapper when done from converted code) and we just need to implement a local or remote LowLevelStream for server side vs client side.

StreamDaemon has a dependency on ThinClient related to pseudo-terminal/currentStream/redirects. I think these will be relevant with server-side batch execution. Do I need to get the logic out of ThinClient to be reused by both server-side and client-side streams?

Galya B wrote:

StreamDaemon has a dependency on ThinClient related to pseudo-terminal/currentStream/redirects. I think these will be relevant with server-side batch execution. Do I need to get the logic out of ThinClient to be reused by both server-side and client-side streams?

No. That processing is very deeply integrated into the UI including redirected terminal, DOWN processing, header (page-top)/footer (page-bottom) frames and so forth. I'd like to defer that to #9055.

Can you abstract the TC dependency in an interface and provide an empty/safe implemention on the server side for now?

Greg Shah wrote:

Galya B wrote:

StreamDaemon has a dependency on ThinClient related to pseudo-terminal/currentStream/redirects. I think these will be relevant with server-side batch execution. Do I need to get the logic out of ThinClient to be reused by both server-side and client-side streams?

No. That processing is very deeply integrated into the UI including redirected terminal, DOWN processing, header (page-top)/footer (page-bottom) frames and so forth. I'd like to defer that to #9055.

Can you abstract the TC dependency in an interface and provide an empty/safe implemention on the server side for now?

I'll try to.

I'm getting back to the same issue: Streams can't be communicated between the server and the clients. The ids were originally generated and stored client-side and mapped to stream objects, which were created and used client-side, not serialized to be sent to the server. The server dealt only with stream ids / names. With server-side streams the stream objects are created server-side. But the tens of stream methods client-side need the streams and the streams can't be simply serialized and moved back and forth.

This means streams and everything related (filesystem, process launcher, environments, etc.) can't be server-side and work with the relevant clients methods.

The ThinClient methods using stream objects client-side are:

choose(int, ChooseData, EventList)
clearWorker(Frame, boolean, boolean, boolean)
deregisterPageElement(int, boolean)
deregisterPageElement(int, boolean, int)
down(int, int, boolean, int)
down(int, int, int, boolean, boolean)
downWorker(int, int, boolean, boolean, boolean)
enable(int, int[], ScreenBuffer[], boolean, int, boolean, int[])
enableWorker(int[], ScreenBuffer[], boolean, Widget, Frame, boolean, int)
endOutput(int)
flushRedirected(Stream, boolean)
flushStream(Stream)
getCurrentRedirection()
honorConditionalUpDown()
honorConditionalUpDown(boolean)
isKeepMessages()
isRedirected(Stream)
notifyAdvance(Stream)
preprocessView(ScreenBuffer[], Widget, int[])
pushScreenDefinition(ScreenDefinition[])
redirectedTerminalForStream(Stream)
redirectOutput(int, int)
refreshFrameWidgets(int, int[], BaseDataType[], boolean)
registerPageElement(int, boolean, boolean)
registerPageElement(int, boolean, int, boolean)
registerPageElement(int, boolean, int, boolean, boolean)
requestHelp()
scroll(int, boolean, boolean)
switchFrames(int, int)
switchStreams(Frame)
switchWorker(int)
underline(int, int[])
view(int)
view(int, ScreenBuffer[], int[], boolean, int, int[])
view(int, ScreenBuffer[], int[], int, boolean, boolean, boolean, ...)
view(Widget, ScreenBuffer[], int[], boolean)
viewWorker(Widget, ScreenBuffer[], int[], boolean, int)
view_(int, ScreenBuffer[], int[], boolean, boolean, int)
waitFor(EventList, int, int, ScreenBuffer[], BlockingOperation, boolean, int)

If there are customers with filesystem enabled server-side, and 4065g enables streams server-side implicitly with the filesystem, then it will render these methods unusable, so any UI work to keep these customer systems running should be merged together with 4065g.

As long as different sessions can have different settings for server-side support, we can retain some sessions with full UI support and client-side filesystem/streams while other sessions run server side. With this in mind, I think we can accept different delivery schedules for 4065g and the results of #9055.

Behind what config should I put the SessionExports that is only date related methods to prevent SessionUtils doing the trip to the client? I can add a new resource config "date", but it doesn't make logical sense to exist on its own.

Galya B wrote:

Behind what config should I put the SessionExports that is only date related methods to prevent SessionUtils doing the trip to the client? I can add a new resource config "date", but it doesn't make logical sense to exist on its own.

Probably "all" should be the one, if no other suggestions.

The date support would be used by the UI, I/O code like export or put with formatted values and I'm not sure what else. The bottom line here: it isn't something that would be setup server side on its own. I agree, it is only safe in the all case.

Hynek: Please review.

I'll rebase.

4065g rebased on trunk r15446. Ready for review.

Fix for #9182 in r15453-r15454: all OS resources client-side should be recreated (initialized) when the security context changes (with stop_disposition = 1).

Hynek, when are you having time to review the branch?

Galya B wrote:

Hynek, when are you having time to review the branch?

I'm on it.

Code review 4065g.

Nice set of changes!

ThinClient history entries are not merged properly. The 4-digit format of ids is gone and there are some missing entries.

In ServerResourceManager, if isServerSide(OsResourceType.STREAMS) the instantiated StreamHelper is not added to the resources map.

In ClientSideManager there are multiple occurrences of "not supported" while in fact the resource is supported. This is confusing.

There is no name for OsResourceType.DATE.

Shouldn't the network objects for the new resource types be registered in ServerResourceManager, not just on-demand?

Hynek Cihlar wrote:

In ClientSideManager there are multiple occurrences of "not supported" while in fact the resource is supported. This is confusing.

This is for resources that will never get exposed by the server to the client with a server-side resource config, because in these cases there is not expected to ever be a call from the client. If eventually something changes and a breakpoint is hit, the comment may help resolve it, that was my intent.

There is no name for OsResourceType.DATE.

Yes, the names in the enum are used to resolve directory configs for enabling resources, while date will not be considered a resource on its own. Nevertheless it has its enum value to hold instanced in the map.

Shouldn't the network objects for the new resource types be registered in ServerResourceManager, not just on-demand?

MEMPTR, LIBRARY, SOCKET have a general rpc server registered server-side, but the other resources are only used in the context of the session, so their instances can only be ContextLocal.

As for the other comments, I'll take a look and come back.

Galya B wrote:

Hynek Cihlar wrote:

In ClientSideManager there are multiple occurrences of "not supported" while in fact the resource is supported. This is confusing.

This is for resources that will never get exposed by the server to the client with a server-side resource config, because in these cases there is not expected to ever be a call from the client. If eventually something changes and a breakpoint is hit, the comment may help resolve it, that was my intent.

If it is not exposed by the server, why is the network instance obtained?

      if (isServerSide(OsResourceType.LAUNCHER))
      {
         // not supported
         launcher = (Launcher) RemoteObject.obtainNetworkInstance(Launcher.class);
      }

There is no name for OsResourceType.DATE.

Yes, the names in the enum are used to resolve directory configs for enabling resources, while date will not be considered a resource on its own. Nevertheless it has its enum value to hold instanced in the map.

OK. In that case please modify the overriden toString. For DATE this will return null.

Shouldn't the network objects for the new resource types be registered in ServerResourceManager, not just on-demand?

MEMPTR, LIBRARY, SOCKET have a general rpc server registered server-side, but the other resources are only used in the context of the session, so their instances can only be ContextLocal.

But for the process launcher, for example, the server is registered, see initializeProcessLauncher. So unless getProcessLauncher is called on the server the client won't be able to obtain the network instance. Please clarify.

Hynek Cihlar wrote:

Galya B wrote:

Hynek Cihlar wrote:

In ClientSideManager there are multiple occurrences of "not supported" while in fact the resource is supported. This is confusing.

This is for resources that will never get exposed by the server to the client with a server-side resource config, because in these cases there is not expected to ever be a call from the client. If eventually something changes and a breakpoint is hit, the comment may help resolve it, that was my intent.

If it is not exposed by the server, why is the network instance obtained?
[...]

I've cleaned it up in r15456.

There is no name for OsResourceType.DATE.

Yes, the names in the enum are used to resolve directory configs for enabling resources, while date will not be considered a resource on its own. Nevertheless it has its enum value to hold instanced in the map.

OK. In that case please modify the overriden toString. For DATE this will return null.

Currently it's returning the name private field, that is null.

Shouldn't the network objects for the new resource types be registered in ServerResourceManager, not just on-demand?

MEMPTR, LIBRARY, SOCKET have a general rpc server registered server-side, but the other resources are only used in the context of the session, so their instances can only be ContextLocal.

But for the process launcher, for example, the server is registered, see initializeProcessLauncher. So unless getProcessLauncher is called on the server the client won't be able to obtain the network instance. Please clarify.

Correct. The launcher is not expected to be called by the client with server-side config enabled.

Hynek Cihlar wrote:

ThinClient history entries are not merged properly. The 4-digit format of ids is gone and there are some missing entries.

There was back and forth with some revisions on trunk and I rebased in the meantime, so I'm not sure what went wrong, but a fix in r15457.

In ServerResourceManager, if isServerSide(OsResourceType.STREAMS) the instantiated StreamHelper is not added to the resources map.

Fixed in r15456.

Let me know if it's good for merge.

hotel_gui was converted and tested for regressions. A customer project was also ran for finding regressions.

If there is a customer with filesystem enabled in server-side-resources in directory, they will get streams auto enabled and this can lead to a change in the behavior, but I'm not aware of a customer having such setup.

Greg, are we merging?

It is Hynek's call for merge, if testing is sufficient.

Hynek, are we merging?

Hynek, this task has been in review for 25 days and is blocking a high priority task #8661. Can you prioritize it?

Please test Hotel ChUI and something in batch mode. I assume you ran conversion on Hotel GUI.

Hotel chui has an issue with the (R)eturn cmd - not working from the Rates screen. This is from the server:

24/10/08 16:41:08.281+0300 |  SEVERE | com.goldencode.p2j.util.TransactionManager | ThreadName:Conversation [00000002:bogus], Session:00000002, ThreadId:00000005, User:bogus | Abnormal end; original error:
java.lang.RuntimeException: invoke() of class com.goldencode.hotel.Rates and  method execute failed
    at com.goldencode.p2j.util.ControlFlowOps.invokeError(ControlFlowOps.java:8453)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7603)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4526)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7107)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7010)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1405)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1387)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1335)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:426)
    at com.goldencode.hotel.Main.lambda$null$1(Main.java:133)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:10518)
    at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:1615)
    at com.goldencode.hotel.Main.lambda$null$2(Main.java:131)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:10518)
    at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:1615)
    at com.goldencode.hotel.Main.lambda$null$4(Main.java:122)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.coreLoop(BlockManager.java:10980)
    at com.goldencode.p2j.util.BlockManager.repeatWorker(BlockManager.java:10874)
    at com.goldencode.p2j.util.BlockManager.repeat(BlockManager.java:2523)
    at com.goldencode.hotel.Main.lambda$execute$5(Main.java:108)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:669)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:642)
    at com.goldencode.hotel.Main.execute(Main.java:76)
    at com.goldencode.hotel.MainMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9676)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9632)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7529)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7544)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:4526)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7107)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7010)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1405)
    at com.goldencode.p2j.util.ControlFlowOps.invokeWithMode(ControlFlowOps.java:1387)
    at com.goldencode.p2j.util.ControlFlowOps.invoke(ControlFlowOps.java:1335)
    at com.goldencode.p2j.util.InvokeConfig.run(InvokeConfig.java:426)
    at com.goldencode.hotel.Login.lambda$execute$1(Login.java:90)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9271)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8892)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:669)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:642)
    at com.goldencode.hotel.Login.execute(Login.java:44)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.goldencode.p2j.util.Utils.invoke(Utils.java:1801)
    at com.goldencode.p2j.main.StandardServer$MainInvoker.execute(StandardServer.java:2908)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2182)
    at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2104)
    at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:698)
    at com.goldencode.p2j.main.StandardServerMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:422)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:235)
    at java.lang.Thread.run(Thread.java:750)
Caused by: com.goldencode.p2j.net.SilentUnwindException: Connection ended abnormally
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:149)
    at com.sun.proxy.$Proxy16.destroyFrame(Unknown Source)
    at com.goldencode.p2j.ui.LogicalTerminal.destroyFrame(LogicalTerminal.java:12928)
    at com.goldencode.p2j.ui.GenericFrame.destroy(GenericFrame.java:7737)
    at com.goldencode.p2j.ui.GenericFrame.frameCleanup(GenericFrame.java:8765)
    at com.goldencode.p2j.ui.FrameWidget.delete(FrameWidget.java:1998)
    at com.goldencode.p2j.util.ProcedureManager.deleteResources(ProcedureManager.java:3054)
    at com.goldencode.p2j.util.ProcedureManager.delete(ProcedureManager.java:2386)
    at com.goldencode.p2j.util.ProcedureManager.delete(ProcedureManager.java:2213)
    at com.goldencode.p2j.util.ProcedureManager$WorkArea.scopeFinished(ProcedureManager.java:5574)
    at com.goldencode.p2j.util.TransactionManager.processScopeNotifications(TransactionManager.java:7856)
    at com.goldencode.p2j.util.TransactionManager.popScope(TransactionManager.java:4788)
    at com.goldencode.p2j.util.TransactionManager.access$7600(TransactionManager.java:735)
    at com.goldencode.p2j.util.TransactionManager$TransactionHelper.popScope(TransactionManager.java:9009)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8974)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:669)
    at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:642)
    at com.goldencode.hotel.Rates.execute(Rates.java:153)
    at com.goldencode.hotel.RatesMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9676)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9632)
    at com.goldencode.p2j.util.ControlFlowOps.lambda$invokeImpl$12(ControlFlowOps.java:7529)
    at com.goldencode.p2j.util.ControlFlowOps.invokeImpl(ControlFlowOps.java:7544)
    ... 64 more
Caused by: java.lang.IllegalStateException: The queue is not running!
    at com.goldencode.p2j.net.Queue.checkState(Queue.java:1366)
    at com.goldencode.p2j.net.Queue.transact(Queue.java:677)
    at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:273)
    at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:221)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1468)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:144)
    ... 86 more
24/10/08 16:41:08.282+0300 |  SEVERE | com.goldencode.p2j.util.TransactionManager [TransactionManager.handleDeferredError] | ThreadName:Conversation [00000002:bogus], Session:00000002, ThreadId:00000005, User:bogus | <depth = 5; trans_level = -1; trans_label = null; rollback_scope = -1; rollback_label = null; rollback_pending = false; in_quit = false; retry_scope = -1; retry_label = null; ignore_err = false> [label = blockLabel0; type = DO; full = false; trans_level = SUB_TRANSACTION; external = false; top_level = false; loop = false; loop_protection = true; had_pause = false; endkey_retry = false; next_or_leave = leave; is_retry = false; needs_retry = false; FOR (aggressive) flushing = false; ilp_count = -1; pending_break = false; database_trigger = false; properties = 'STOP'; finally = none] Throwing deferred error (Connection ended abnormally)

The client shows some warnings:

24/10/08 16:36:24.325+0300 | WARNING | com.goldencode.p2j.ui.client.event.EventManager | PID:259230, ThreadName:main, Session:00000003, User:bogus | The unicode codepoint '501' cannot be represented by this codeset 'ISO-8859-1'

I don't see any link to my changes. Probably a misconfig. The other funtionality seems to work in web and swing. I'm not sure how to enter user/pass in terminal, or more specifically why it doesn't work (quit worked).

Galya B wrote:

I'm not sure how to enter user/pass in terminal, or more specifically why it doesn't work (quit worked).

I've figured it out. Same behavior.

If there is no difference from trunk, we're good.

BTW, I tried Hotel ChUI last week with trunk and it did work, so I agree it might be a configuration/setup issue.

Also to confirm: batch mode is OK?

Greg Shah wrote:

Also to confirm: batch mode is OK?

I've tested in batch with simple procedures writing to streams.

Sorry for the delays on this issue, but I'm not getting any email notifications on this. For some reason Thunderbird hides these in my inbox. I can lookup the emails when using the search feature, but the thread is marked as ignored and can't be unmarked.