Project

General

Profile

Bug #4221

Error in BaseEntity class

Added by Vladimir Tsichevski over 4 years ago. Updated over 4 years ago.

Status:
Review
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

80%

billable:
No
vendor_id:
GCD
case_num:

History

#1 Updated by Vladimir Tsichevski over 4 years ago

The code below

define variable num  as integer.
display num with frame f0.
num = num:dcolor.

Does not work in FWD swing (the application window appears and disappears endlessly).

The same code causes no problem in Progress.

The cause of the problem is NPE in the com.goldencode.p2j.ui.BaseEntity.getDColor() method:

   public integer getDColor()
   {
      int color = config.dcolor.color();
      return (color == -1) ? new integer() : new integer(color);
   }

since the config.dcolor field is null.

Changing the method code to the following one cures the problem:

   public integer getDColor()
   {
      Color dcolor = config.dcolor;
      return (dcolor == null)
         ? new integer()
         : new integer(dcolor.color());
   }

#2 Updated by Hynek Cihlar over 4 years ago

Vladimir Tsichevski wrote:

The code below

[...]

Does not work in FWD swing (the application window appears and disappears endlessly).

The same code causes no problem in Progress.

The cause of the problem is NPE in the com.goldencode.p2j.ui.BaseEntity.getDColor() method:

[...]

since the config.dcolor field is null.

Changing the method code to the following one cures the problem:

[...]

Was removing the case color == -1 intentional?

#3 Updated by Vladimir Tsichevski over 4 years ago

Hynek Cihlar wrote:

Was removing the case color == -1 intentional?

No, this was my mistake. Please, restore the handling of the -1 value.

#4 Updated by Greg Shah over 4 years ago

  • Status changed from New to WIP
  • Assignee set to Vladimir Tsichevski

Vladimir: Please put the change (including the -1 handling) into branch 4124a.

Also, please check the other color attributes (PFCOLOR, FGCOLOR, BGCOLOR) to ensure they don't have the same problem.

#5 Updated by Vladimir Tsichevski over 4 years ago

Greg Shah wrote:

Vladimir: Please put the change (including the -1 handling) into branch 4124a.

Also, please check the other color attributes (PFCOLOR, FGCOLOR, BGCOLOR) to ensure they don't have the same problem.

Things to be fixed either:

1. The setDColor(NumberType param) causes explicitly constructed NPE (in param.intValue()), if an "unknown" integer is passed as the method argument:

num:dcolor = ?.

To fix this, the method implementation shall be changed from:

   public void setDColor(NumberType param)
   {
      config.dcolor = new ColorSpec(param.intValue()).convert();
      pushWidgetAttr("dcolor", config.dcolor);
   }

to

   public void setDColor(NumberType param)
   {
      int color = (param == null || param.isUnknown()) ? -1 : param.intValue();
      config.dcolor = new ColorSpec(color).convert();
      pushWidgetAttr("dcolor", config.dcolor);
   }

2. The code:

num:pfcolor = ?.

currently causes the NPE by the same reason:

   public void setPfColor(NumberType param)
   {
      config.pfcolor = new ColorSpec(param.intValue()).convert();
      pushWidgetAttr("pfcolor", config.pfcolor);
   }

shall be probably changed to:

   public void setPfColor(NumberType param)
   {
      int color = (param == null || param.isUnknown()) ? -1 : param.intValue();
      config.pfcolor = new ColorSpec(color).convert();
      pushWidgetAttr("pfcolor", config.pfcolor);
   }

Another implication is that Progress allows neither read nor write the pfcolor with the following message:

**Progress does not support the PFCOLOR attribute for FILL-IN num in this display environment. (4088)

FWD does not display anything similar.

I am not sure what shall I do here.

#6 Updated by Hynek Cihlar over 4 years ago

Vladimir Tsichevski wrote:

FWD does not display anything similar.

I am not sure what shall I do here.

You get this error because the PFCOLOR attribute is supported only in ChUI session while you ran the app in GUI. See the method LogicalTerminal.isChui, this will return true when in ChUI. To display the error, use ErrorManager.recordOrShowError. You can see both methods in action for example in BaseEntity.setXorY().

#7 Updated by Vladimir Tsichevski over 4 years ago

Hynek Cihlar wrote:

Vladimir Tsichevski wrote:

FWD does not display anything similar.

I am not sure what shall I do here.

You get this error because the PFCOLOR attribute is supported only in ChUI session while you ran the app in GUI. See the method LogicalTerminal.isChui, this will return true when in ChUI. To display the error, use ErrorManager.recordOrShowError. You can see both methods in action for example in BaseEntity.setXorY().

I run in GUI mode both FWD (gui_swing) and Progress. I see the message in Progress, and I do not see it in FWD.

So, I need to

1. Check if I am in terminal mode with LogicalTerminal.isChui();
2. If not, refuse both getting and setting attempts with the message above.

Is that correct?

#8 Updated by Hynek Cihlar over 4 years ago

Vladimir Tsichevski wrote:

Hynek Cihlar wrote:

Vladimir Tsichevski wrote:

FWD does not display anything similar.

I am not sure what shall I do here.

You get this error because the PFCOLOR attribute is supported only in ChUI session while you ran the app in GUI. See the method LogicalTerminal.isChui, this will return true when in ChUI. To display the error, use ErrorManager.recordOrShowError. You can see both methods in action for example in BaseEntity.setXorY().

I run in GUI mode both FWD (gui_swing) and Progress. I see the message in Progress, and I do not see it in FWD.

So, I need to

1. Check if I am in terminal mode with LogicalTerminal.isChui();
2. If not, refuse both getting and setting attempts with the message above.

Is that correct?

Correct. Just check the behavior for both attribute read and write in OpenEdge.

#9 Updated by Vladimir Tsichevski over 4 years ago

Hynek Cihlar wrote:

Correct. Just check the behavior for both attribute read and write in OpenEdge.

The same error message is displayed both for read and write of pfcolor in OpenEdge.

#10 Updated by Vladimir Tsichevski over 4 years ago

Vladimir Tsichevski wrote:

The same error message is displayed both for read and write of pfcolor in OpenEdge.

Correction: OpenEdge emits the warning on the write only, and it does not prevent either reading or writing operation.

#11 Updated by Vladimir Tsichevski over 4 years ago

Correction 2: when in GUI, OpenEdge silently does not write the dcolor attribute, and reading the dcolor silently returns undefined value.

So, despite the OpenEdge docs say neither dcolor nor pfcolor are used in GUI mode, the treatment of this attributes differs in OpenEdge.

#12 Updated by Vladimir Tsichevski over 4 years ago

New observation: OpenEdge refuses to set both fgcolor and bgcolor to any negative value.

In FWD:

num:fgcolor = -1.
num:bgcolor = -1.

both set the corresponding color to undefined.

num:bgcolor = -2.

just sets the bgcolor to -2.

#13 Updated by Vladimir Tsichevski over 4 years ago

1. In any mode OpenEdge refuses to set any color indices greater than 255 with the following message:

Invalid color index: 256 is greater than the current max value. (4086)

2. In Character mode OpenEdge silently ignores the new color value after checking it for validity for fgcolor and bgcolor.

#14 Updated by Vladimir Tsichevski over 4 years ago

Exception: In Character mode OpenEdge only does not complain about the invalid color index for bgcolor :(

#15 Updated by Vladimir Tsichevski over 4 years ago

The Progress vs. FWD operations summarized in the following table:

Color Operation Mode Result in Progress Result in FWD
dcolor get gui always ? same
dcolor set to 0..255 gui nop same
dcolor set to out of 0..255 gui nop same
dcolor set to < 0 gui emit "Attribute XXX must be greater than..." same
pfcolor get gui initially ? or the value previously set always ? (see note below)
pfcolor set to 0..255 gui emit "Progress does not support...", and set the value. emit "Progress does not support...", and set the value to ?.
pfcolor set to >255 gui emit "Invalid color index...". same
pfcolor set to 65535 gui emit "Invalid color index 0...". same
pfcolor set to <0 gui emit "Attribute XXX must be greater than...". same
bgcolor get gui initially ? or the value previously set same
bgcolor set to 0..255 gui set the value same
bgcolor set to >255 gui emit "Invalid color index...". same
bgcolor set to 65535 gui emit "Invalid color index 0...". same
bgcolor set to <0 gui emit "Attribute XXX must be greater than...". same
fgcolor <ani> gui same as bgcolor same
dcolor get chui the value previously set initial ?, if set - always 0
dcolor set to 0..255 chui set the value set to 0
dcolor set to > 255 chui emit "Invalid color index..." same
dcolor set to 65535 chui emit "Invalid color index 0 ..." same
dcolor set to < 0 chui emit "Attribute XXX must be greater than..." same
pfcolor get chui initially ?, then the value previously set initially ?, after set - always 0
pfcolor set to 0..255 chui set the value. nop
pfcolor set to > 255 chui emit "Invalid color index..." same
pfcolor set to 65535 chui emit "Invalid color index 0..." same
pfcolor set to < 0 chui emit "Attribute XXX must be greater than..." same
bgcolor get chui always ? same
bgcolor set to 0..255 chui nop same
bgcolor set to > 255 chui emit "Invalid color index..." same
bgcolor set to 65535 chui emit "Invalid color index 0..." same
bgcolor set to out of 0..255 chui nop same
bgcolor set to < 0 chui emit "Attribute XXX must be greater than..." same
fgcolor get chui always ? same
fgcolor set to < 0 chui emit "Attribute XXX must be greater than..." same
fgcolor set to >= 0 chui nop same

The message mentioned in the table above are:

1.

Invalid color index: 256 is greater than the current max value. (4086)

2.

Progress does not support the PFCOLOR attribute for the FILL-IN num in this display environment. (4088)

3.

Attribute BGCOLOR must be greater than or equal to 0 in FILL-IN num. (4082)

Note1: the fgcolor and bgcolor behave differently in Character mode.
Note2: 2-byte integers are used to display invalid color indices, so if you try num:dcolor = 65536, the warning will look like:

Invalid color index: 0 is greater than the current max value. (4086)

#16 Updated by Vladimir Tsichevski over 4 years ago

  • % Done changed from 0 to 80

Changes committed to 4124a, new revision number 11451.

#17 Updated by Vladimir Tsichevski over 4 years ago

At the moment I cannot reach 100% compatibility with Progress in that how the dcolor and pfcolor colts are treated, at least, in GUI mode.

The reason is that it seems FWD does not keep the original integer color value for these slots, and tries to convert them to Color objects.

The method Color ColorSpec.convert(ColorSpec cs) in GUI mode looks like the following:

      public Color convert(ColorSpec cs)
      {
         GUIWorkArea wa = locate();
         Color color = null;
         if (cs.colorValue instanceof String)
         {
            ... process color name here
         }

         return color;      

      }

So, anything but Strings, is silently ignored, and the method always return null.

But currently the color setters look like the following (this code is before I changed it a bit, but that does not matter in this case):

   public void setPfColor(NumberType param)
   {
      config.pfcolor = new ColorSpec(param.intValue()).convert();
      pushWidgetAttr("pfcolor", config.pfcolor);
   }

So they always set the config.dcolor and config.pfcolor to null when in GUI mode.

Meanwhile the Progress allows to change these values even when in GUI mode, and the update values can be read back.

Probably we need to make the dcolor and pfcolor handling after than for fgcolor and bgcolor, and always keep the color indices as integers? This is to achieve the full compatibility only, the values themselves are not used by FWD run time.

#18 Updated by Eugenie Lyzenko over 4 years ago

The change here 11451 or 11452 causes the StackOverflow for endless com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798) calls in a customer application.

#19 Updated by Eugenie Lyzenko over 4 years ago

Server.log for StackOverflow.

[08/14/2019 02:48:02 MSK] (com.goldencode.p2j.util.TransactionManager:SEVERE) Abnormal end; original error:
java.lang.StackOverflowError
        at java.lang.ThreadLocal.get(ThreadLocal.java:161)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:469)
        at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:430)
        at com.goldencode.p2j.main.StandardServer.getClientParameters(StandardServer.java:731)
        at com.goldencode.p2j.ui.LogicalTerminal.isChui(LogicalTerminal.java:8870)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:796)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
        at com.goldencode.p2j.ui.BaseEntity.setBgColor(BaseEntity.java:798)
...

#20 Updated by Eugenie Lyzenko over 4 years ago

The following code in BaseEntity causes endless recursive loop in GUI mode:

   public void setBgColor(integer bgcolor)
   {
      if (bgcolor == null || bgcolor.isUnknown())
      {
         if (!LogicalTerminal.isChui())
         {
            setBgColor(new integer()); <---- endless call to myself
         }

         return;
      }
...
   public void setFgColor(integer fgcolor)
   {
      if (fgcolor == null || fgcolor.isUnknown())
      {
         setFgColor(new integer()); <---- endless call to myself
         return;
      }
...
   public void setDColor(NumberType param)
   {
      if(param == null || param.isUnknown())
      {
         setDColor(new integer()); <---- endless call to myself
         return;
      }
...

If you will not able to commit the change in 30 min I'll make the fix and commit. Because it is stopper for me now.

#21 Updated by Eugenie Lyzenko over 4 years ago

Task branch 4124a has been updated for review to revision 11453.

This is the fix for StackOverflow from endless recursion and another NPE in FillIn painting code.

Note the UNKNOWN(?) Progress value does not mean the object is NULL in FWD world.

#22 Updated by Vladimir Tsichevski over 4 years ago

Eugenie Lyzenko wrote:

Task branch 4124a has been updated for review to revision 11453.

Ok, thank, you were first.

Note the UNKNOWN(?) Progress value does not mean the object is NULL in FWD world.

I know this, thanks.

#23 Updated by Vladimir Tsichevski over 4 years ago

  • Status changed from WIP to Review

Also available in: Atom PDF