FWD » Core Development » Base Language

Guys, there is an issue with scoping on app server. Consider a simple testcase:

def temp-table tt
     field field-one as integer
     field field-two as char.

create tt.
tt.field-one = 3. tt.field-two = "field 3".
create tt.
tt.field-one = 4. tt.field-two = "field 4".

message "appserver external program 2 ran".
for each tt:
   message string(tt.field-one) + "    " + string(tt.field-two).
end.

It does the job, but finishes with two warnings and exception:

[04/05/2014 12:52:07 TMT] (com.goldencode.p2j.persist.trigger.DatabaseTriggerManager:WARNING) Invalid scope finished.
[04/05/2014 12:52:07 TMT] (com.goldencode.p2j.persist.trigger.DatabaseTriggerManager:WARNING) Invalid scope finished.
[04/05/2014 12:52:07 TMT] (com.goldencode.p2j.util.Agent:WARNING) Agent encountered an error while executing a command for appserver app_server
java.lang.NullPointerException
        at com.goldencode.p2j.persist.BufferManager.scopeDeleted(BufferManager.java:808)
        at com.goldencode.p2j.util.ProcedureManager.delete(ProcedureManager.java:1087)
        at com.goldencode.p2j.util.ExternalProgramWrapper.delete(ExternalProgramWrapper.java:735)
        at com.goldencode.p2j.util.Agent$9.execute(Agent.java:1131)
        at com.goldencode.p2j.util.Agent.listen(Agent.java:349)
        at com.goldencode.p2j.util.AgentPool.start(AgentPool.java:409)
        at com.goldencode.p2j.util.AppServerManager.startAppServer(AppServerManager.java:766)
        at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:265)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:601)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:693)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:319)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:163)
        at java.lang.Thread.run(Thread.java:722)

The issue is about the global scope which is popped by scopeFinished, and when scopeDeleted comes into play loadedBuffers has no scopes.

public void scopeDeleted()
   {
      Object referent = ProcedureManager.getProcessedProcedure();
      PersistentProcScope scope = persistProcScopes.get(referent);
      // this needs to be executed only when the procedure gets deleted.
      if (referent != null && scope != null && !ProcedureManager._isPersistent(referent))
      {
         // remove all these from the global scope
         loadedBuffers.getDictionaryAtScope(loadedBuffers.size() - 1, true)
                      .keySet().removeAll(scope.loadedBuffers.keySet());

Simple loadedBuffers.size() > 0 check works as workaround for me, but warnings remain.
I'm not sure how this code is supposed to work.

Guys, consider an external persistent procedure and a buffer scoped to that procedure. When we call an internal procedure from that external procedure which references that buffer then we do not have a proper record for that buffer in allBuffers. I see that that contents of allBuffers is saved into PersistentProcScope.allBuffers on external procedure call, but I cannot find where they are restored (should they?) when internal one is called. Note that topmost scope for the external procedure is also deleted.

Stanislav Lomany wrote:

Note that topmost scope for the external procedure is also deleted.

I think the problem is here. When an appserver procedure is ran persistent and its topmost scope is finished, I think BufferManager needs to be aware of this and keep the global (and current?) scope for the ScopedDictionary variables.

When a persistent proc is deleted and BufferManager.scopeDeleted is called, this needs to remove all the data saved in the PersistentProcScope instance from the BufferManager - see the scopeDeleted implementation.

I think the problem is here. When an appserver procedure is ran persistent and its topmost scope is finished, I think BufferManager needs to be aware of this and keep the global (and current?) scope for the ScopedDictionary variables.

The scope which is referenced as "global" in the comments

// add all these to the global scope, too
allBuffers.getDictionaryAtScope(allBuffers.size() - 1, true).putAll(scope.allBuffers);

is actually the "startup" scope:

// add the topmost scope to the TransactionManager
TransactionManager.pushScope("startup", TransactionManager.NO_TRANSACTION,...);
...
// remove the topmost scope from the TransactionManager
TransactionManager.popScope();

Where should we "keep" the values - in the global scopes (should we add them?) of scoped dictionaries inside BufferManager or separately in PersistentProcScope?

Stanislav Lomany wrote:

Where should we "keep" the values - in the global scopes (should we add them?) of scoped dictionaries inside BufferManager or separately in PersistentProcScope?

1. at the end of Agent.prepare, use TM.pushScope to push an appserver-agent scope (similar to the "startup" scope).
2. in Agent$ResetContextCommand.execute, before calling resetContext, call TM.popScope to pop the appserver-agent scope.

This will allow a global, per-agent, scope, in which the surviving buffers can be "leaked".

Constantin, is there a simple way do determine that the specific context is run using an agent?

Stanislav Lomany wrote:

Constantin, is there a simple way do determine that the specific context is run using an agent?

Yes, use AppServerManager.isRemote - it will return true if this context is for an agent.

Stas, I apologize it has taken so long for me to review this. Because of this delay, it is no longer in sync with the latest bzr revision; please sync it up.

Constantin: The update looks OK to me, but I am out of my depth with the appserver implications. Once Stas has sync'd to the latest revision, please review also.

Stanislav Lomany wrote:

Merged with the latest revision.

Stanislav, the logic looks good to me.