Thursday, March 3, 2016

CodeSOD: Widgety Gadgety Boo

“The road of life twists and turns and no two directions are ever the same. Yet our lessons come from the journey, not the destination.” - Don Williams, Jr. (4719290483)

Ancient Coder submits for our review the following migraine-inducer:

This was a bug report about a very minor issue, a little warning box was not popping up. Simple, I thought, and I estimated it would take an hour to fix. Little did I know that someone had already poked at this issue and made this amazing log entry. I spent about half an hour trying to understand this code before I backed away, slowly.

Are you ready for this one, guys? I mean, really ready? OK, here we go:

This problem is caused by BUGREP-92726, but it is more complex problem. Actually it is caused by this part of code:

ForgivingSuper.java:1435
int lastWidget = WebThread.getWidgetHistoryMgr().getPreviousWidgetId();
// BUGREP-53139 03/24/2014 gmp - check whether previous ErrorGadget is 337, if it is do not put up ErrorGadget 32
ErrorGadgetWidget prevErrorGadget = WebThread.getPartialWidgetMgr()!=null?WebThread.getPartialWidgetMgr().getPreviousErrorGadget():null;
if((lastWidget == IWidgetIds.PROGRAM_INFO_Widget && WebThread.getErrorGadgetManager().getErrorGadget32OverrideStatus()) || 
    (((currentErrorGadget != null && currentErrorGadget.getErrorGadgetNumber() == ErrorGadget337) || 
    (prevErrorGadget != null && prevErrorGadget.getErrorGadgetNumber() == ErrorGadget337))))
{
    // WebThread.getErrorGadgetManager().removeErrorGadget(ErrorGadget32);
    ErrorGadgetContext = new ErrorGadgetContext();
    ErrorGadgetContext.put(ErrorGadget337_DATA_SERVICE_EVENT, se);
    ErrorGadgetContext.put(ErrorGadget337_Defeat_OPTIONS, prevErrorGadget.getContext().get(ErrorGadget337_Defeat_OPTIONS));
    WebThread.getErrorGadgetManager().showErrorGadget(ErrorGadget337, ErrorGadgetContext);
    WebThread.getErrorGadgetManager().setErrorGadget32OverrideStatus(false);
} else {
    // BUGREP-64543 03/24/2014 gmp - show ErrorGadget 32 since it is not overridden by ErrorGadget 337
    ErrorGadgetContext.put(ErrorGadget32_DATA_SERVICE_EVENT, se);
    ErrorGadgetContext.put(ErrorGadget32_IS_BOOKED, WebThread.getSharedAlgorithms().isScheduledToShow(se));
    curErrorGadgetManager.showErrorGadget(ErrorGadget32, ErrorGadgetContext);
}

This part of code is patch applied for BUGREP-92726 and it solves problem describe there, but only partially. As it creates another problems:
1. When above patch code is applied and we steps of reproduce BUGREP-92726 + go to report from used in steps of reproduce of BUGREP-92726, then we still see ErrorGadget337 despite that we expect ErrorGadget32 (as we swapped to other DataEvent).
It is because we go into following condition of above code snippet:

(prevErrorGadget != null && prevErrorGadget.getErrorGadgetNumber() == ErrorGadget337)

2. This can cause another similar problems every time when prevErrorGadget was ErrorGadget337. I think that, the if statement from the above snippet should be as simple as:

if(WebThread.getErrorGadgetManager().getErrorGadget32OverrideStatus())
{
        .....
} else {
   .....
}

However in such situation we have another problems to solve:
1. WebThread.getErrorGadgetManager().getErrorGadget32OverrideStatus() is not properly set throughout the code. It should improved.
2. prevErrorGadget can be null => following issue will return NullPointerException(): ErrorGadgetContext.put(ErrorGadget337_Defeat_OPTIONS, prevErrorGadget.getContext().get(ErrorGadget337_Defeat_OPTIONS));
3. Even if prevErrorGadget is not null, there are situations where prevErrorGadget.getContext().get(ErrorGadget337_Defeat_OPTIONS) returns Boolean object instead of DefeatOption object.

It is when we reproducing this issue (BUGREP-47765). Then following exception is caught:

|Oct 22 11:22:04  java.lang.IllegalArgumentException: Failed to build ErrorGadget337: com.parent.mw.ca.dat.DefeatOption object is expected.
 Refer to the javadoc for ErrorGadgetConstants.ErrorGadget337_XXX for details. Object received: java.lang.Boolean       
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetTextBuilder.getObject (Unknown Source, bco=79)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetTextBuilder.buildErrorGadgetText (Unknown Source, bco=16486)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetBuilder.buildTextBlock (Unknown Source, bco=9)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetBuilder.buildErrorGadget (Unknown Source, bco=112)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetManager.buildErrorGadget (Unknown Source, bco=201)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.ErrorGadget.ErrorGadgetManager.showErrorGadget (Unknown Source, bco=15)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.SystemEventMgr.handleViewingProblems (Unknown Source, bco=3327)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.SystemEventMgr.handleViewingProblems (Unknown Source, bco=6)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.SystemEventMgr$MyServiceEventListener$1.run (Unknown Source, bco=16)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.EventManager.invokeAndWait (Unknown Source, bco=41)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.EventManager$2.run (Unknown Source, bco=8)
|Oct 22 11:22:04      at com.parent.finagler.nestedobsoletestructure.EventManager$TaskWrapper.run (Unknown Source, bco=53)
|Oct 22 11:22:04      at ceej.util.ThreadPoolEntry.run (Unknown Source, bco=55)
[Advertisement] BuildMaster integrates with an ever-growing list of tools to automate and facilitate everything from continuous integration to database change scripts to production deployments. Interested? Learn more about BuildMaster!


To read the full article, please visit The Daily WTF

No comments:

Post a Comment