FWD » Core Development » Database

So, what we need to do is to replace the workaround of just combining all listeners sets and instead use the ListenerSet's iterator?

Razvan-Nicolae Chichirau wrote:

So, what we need to do is to replace the workaround of just combining all listeners sets and instead use the ListenerSet's iterator?

Drop the workaround and find why the ListenerSet$Iter is not doing its job right.

Given this situation:

DEFINE VARIABLE q AS HANDLE NO-UNDO.
PROCEDURE openQuery:
...
CREATE QUERY q.
q:QUERY-OPEN().
...
END PROCEDURE.

ON CHOOSE OF btn DO:
    ghBuffer:BUFFER-DELETE(). 
    q:QUERY-CLOSE(). 
    q:QUERY-OPEN().
END.

// Main block
RUN openQuery.
wait-for ...

1. The openQuery procedure is invoked. This will trigger the 2 listener sets containing TemporaryBuffer and PreselectQuery to be linked.
2. Procedure is finished, TransactionManager@TransactionHelper.popScope() is called which leads to ChangeBroker.scopeFinished() that unlinks all listener sets returned by listeners.getDictionaryAtScope(0, false), including the set with PreselectQuery.
3. On choosing the button, .deleteRecord() is invoked, leading to the for-each which uses ListenerSet$Iter, but the 2 listener sets are still unlinked, so only one gets picked up.
4. Query open executes again, linking TemporaryBuffer with PreselectQuery, but now it's too late.

So, I don't think the problem comes from the unlinking of PreselectQuery after popping the procedure local scope, but rather why the listener set from the first scope is not linked with the rest.

I agree with this statement.

Committed to 9914a/rev. 15868.

The same code is inside CompoundQuery. This approach should be also applied there. However, I don't think the increasing of scope is the right thing to do here. It looks like you are registering the query at the first nested scope and the buffer at the second nested scope. I suppose the query won't be global anymore.

For the original problem, I tend to think that the global scope (-1) is populated before the local scope (0) in your case.

Note how ListenerSet nextInner = listeners.reverseLookup(dmoIface, 1); is linking the global scope with the first nested scope. However, ListenerSet nextOuter = listeners.lookup(dmoIface, depth - 1); is not ever linking the global scope with the nested scope. There is an invariant here that the listeners are always registered in increasing order of the scope and if the global scope is ever registered, it will be linked with the first nested scope. I think the bug you are seeing here is the invariant violation: a nested scoped set is created after the global scope.

Please confirm that this is the case. If so, please make the required changes to link the global scope if any to the fresh nested scope but only if there is no previous scope. An else to if (set.nextOuter == null && depth > 1) should look for listeners.getValueAtScope(dmoIface, -1); and if it exists, then link to it.

It looks like you are registering the query at the first nested scope and the buffer at the second nested scope.

The scope increasing was due to treating a 0-indexed value as 1-indexed. More exactly, in

int scope = BufferManager.get().findNearestExternal();

Alex: Is this difference in scope wanted? If it's indeed a bug, does #9914-9 still hold?

I can't tell in #9914-5 where the buffer is defined.

After converting start.p from #9914-6:

public void execute()
{
   externalProcedure(Start.this, new Block((Body) () -> 
   {
      fmainFrame.openScope();
      RecordBuffer.openScope(tt1);
      tt1.create();
      tt1.setF1(new character("a"));
      RUN_CALL_SITE_1.clone().run();
      apply(gwhBrowse, "entry");

      registerTrigger(new EventList("choose", fmainFrame.widgetBtnd()), Start.this, new Trigger((Body) () -> 
      {
         RUN_CALL_SITE_2.clone().run();
      }));

      fmainFrame.enable();

      LogicalTerminal.waitFor(null, new EventList("close", currentWindow()));
   }));
}

The place where the TemporaryBuffer is registered as a listener is RecordBuffer.openScope(tt1);, where tt1 is the default buffer for the temp-table, so I don't see any linking with the actual buffer variable. Anyway, it would be defined globally.

Anyway, it would be defined globally.

I am not sure that "globally" is right. It is within the scope of the external procedure, so I would expect to be a non-global scope. Because you run start.p as entry-point, it might get confusing. But technically, if you run a separate procedure from which you do RUN start.p, then tt1 buffer scope is even more nested. The idea is that after RUN finishes, the scope closes and the buffer scope is closed. Finally, this means that the scope is not global.

By "global" I would rather refer to buffers that are CREATEd and which should be deleted explicitly. These buffers persist beyond the scope of the procedure, so beyond RUN. The same applies to other resources like the QUERY.

• the buffer should be at scope 1.
• the query should be at scope 0.

• the buffer should be at scope 2.
• the query should be at scope 0.

• Scope 0 (global): null dictionary
• Scope 1 (custom.p outer scope): null dictionary
• Scope 2 (start.p outer scope): TemporaryBuffer as a listener
• Scope 3 (start.p createWidgets procedure): local PreselectQuery as a listener

No matter how many nested RUN statements we have in this testcase, we always end up trying to register the query at the buffer's scope - 1, which is not equal to the global state (0). Why would we change the addListener implementation if we try to register the query at the wrong scope a.k.a. not the global scope?

What I'm trying to say is that the problem could be from the scope processing, which does not return 0 in this case.

Alex: Can a valid solution be the assigning of a 4GL dynamic query as a global scope listener, according to #9914-13? Note that static and dynamic query scopes are calculated in the same manner (min between the nearest external block and the outer-most record buffer of the query).

No matter how many nested RUN statements we have in this testcase, we always end up trying to register the query at the buffer's scope - 1, which is not equal to the global state (0). Why would we change the addListener implementation if we try to register the query at the wrong scope a.k.a. not the global scope?

I think this is correct for static queries. But I can agree that the query should be registered at scope 0 if dynamic.
Fun fact: a query that had a dynamic buffer (aka with its scope 0) will be automatically registered globally (due to the min operation).

Alex: Can a valid solution be the assigning of a 4GL dynamic query as a global scope listener, according to #9914-13? Note that static and dynamic query scopes are calculated in the same manner (min between the nearest external block and the outer-most record buffer of the query).

Well, if the query is static, the scoping from #9914-14 would be correct! But the issue will still reproduce (?).

I personally see two orthogonal issues, but most important is why some listeners are ignored. We can struggle to find examples with static or dynamic resources that will yield a problematic scoping where the listener set will still fail.
My bottom line is: if we change the scoping of the dynamic query to 0 (which will be correct), I guess we will only hide the issue. Other code can still encounter a malfunction of the listener sets: using static/dynamic resource / persistent procedures / etc. Am I misunderstanding?

Well, if the query is static, the scoping from #9914-14 would be correct! But the issue will still reproduce (?).

You're right. In the static case, the listener sets will not be linked. Commited to rev. 15869.

Alex: Please assess and review.

ETF testing passed!

• ChUI regression tests passed ✅
• Smoke-test for a large customer app passed ✅
• #9722 testing passed ✅

I think we can move on to the second testing phase.

I will run unit tests and smoke tests of large customer application.

Everything is OK here.

• ask Lorian to run harness

Harness passed ✅

• ask Danut to run unit tests and fwdtests for the application he is managing

No issues found ✅

Wonderful! I shall queue the branch up for merging.