Project

General

Profile

Bug #11715

NullPointerException on frame handle assignation

Added by Teodor Gorghe 6 days ago. Updated 2 days ago.

Status:
Test
Priority:
Normal
Assignee:
Teodor Gorghe
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Constantin Asofiei
production:
No
env_name:
topics:

History

#1 Updated by Teodor Gorghe 6 days ago

It's the regression spotted on #11673-14.

This is the testcase:

#3 Updated by Teodor Gorghe 6 days ago

  • Status changed from New to WIP
  • Assignee set to Teodor Gorghe
  • % Done changed from 0 to 100
  • reviewer Constantin Asofiei added
Created task branch 11715a and committed revision 16693:
  • Added laidOut which tells that the doLayout has at least executed once.

#4 Updated by Teodor Gorghe 6 days ago

  • Status changed from WIP to Review

Constantin, can you take a look into 11715a changes?
This path is more reachable from 11394b because frame defs are now lazy and arrive in a different order.

#5 Updated by Greg Shah 6 days ago

#6 Updated by Teodor Gorghe 6 days ago

Done

#7 Updated by Constantin Asofiei 6 days ago

Teodor, what's the stacktrace on the server side when the NPE happens?

#8 Updated by Teodor Gorghe 6 days ago

Stacktrace from logs doesn't tell that much where it happens on the server-side, but I have traced it through debugging and this is:
  • From the converted line: frame my_frame2:frame = frame my_frame:handle -> myFrame2Frame.asWidget().setFrameHandle(myFrameFrame.asWidgetHandle())
  • Goes through FrameWidget.setFrame -> addDynamicWidget -> LT.attachRuntimeWidget and reaches the new code added in #11394: if (!frame.clientRegistered) pushScreenDefinition.
  • abends then in LT.clientCall(), in the processDeferredPush(true); because the parent frame is incomplete because of the previous pushScreenDefinition for the child frame.
  • make 11715a changes to allow frame registration in that order.

#9 Updated by Constantin Asofiei 6 days ago

Teodor, is this something that affects Hotel ChUI?

#10 Updated by Teodor Gorghe 6 days ago

Yes.

#11 Updated by Constantin Asofiei 6 days ago

I've run your test and I don't get the NPE with latest trunk.

#12 Updated by Teodor Gorghe 6 days ago

I ran it from ChUI, with enhanced browse feature turned on.

#13 Updated by Teodor Gorghe 6 days ago

And you need to run test1_driver.p.

#14 Updated by Constantin Asofiei 6 days ago

OK, I see the NPE. The root cause is because the child frame is pushed after it is attached to the parent frame. So, I think the right fix is to push the child frame def before the parent's:

In LT.attachRuntimeWidget, do this before the parent frame:

      if (widget instanceof FrameWidget && !((FrameWidget) widget).frame.isClientRegistered())
      {
         ((FrameWidget) widget).frame.pushScreenDefinition();
      }

      if (!frame.isClientRegistered())
      {
         frame.pushScreenDefinition(true);
      }

Please check and if it makes sense, I think we can merge it.

#15 Updated by Teodor Gorghe 6 days ago

Yes, so you want to keep the order of frame push the same as prior 11394b.

I have already attempted that with something like this:

      if (!frame.isClientRegistered())
      {
         processDeferredPush(true);
         if (!frame.isClientRegistered())
         {
            frame.pushScreenDefinition(true);
         }
      }

but this didn't guaranteed the fact the parent frame is in lt.definitions or lt.level2definitions.

I think the proposed change makes sense, but let me take a look on what can happen.

#16 Updated by Teodor Gorghe 6 days ago

Done in 11715/r16694.

I don't see any issue with this suggestion, widget.frame can't be null here and I think we shouldn't do an ancestor walk here.

#17 Updated by Constantin Asofiei 2 days ago

  • Status changed from Review to Merge Pending

Teodor, please merge 11715a

#18 Updated by Teodor Gorghe 2 days ago

  • Status changed from Merge Pending to Test

Branch 11715a was merged into trunk as rev. 16696 and archived.

Also available in: Atom PDF