Dilbert





The Daily WTF

Error'd: The Illusion of Choice;

"So I can keep my current language setting or switch to Pakistani English. THERE IS NO IN-BETWEEN," Robert K. writes.

 

"I guess robot bears aren't allowed to have the honey, or register the warranty on their trailer hitch" wrote Charles R.

 

"Not to be outdone by King's Cross's platform 0 [and fictional platform 9 3/4], it looks like Marylebone is jumping on the weird band-wagon," David L. writes.

 

Alex wrote, "If the percentage it to be believed, I'm downloading Notepad+++++++++++++++."

 

"Hmm, so many choices?" writes Dave A.

 

Ergin S. writes, "My card number starts with 36 and is 14 digits long so it might take me a little while to get there, but thanks to the dev for at least trying to make things more convenient."

 

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!


Representative Line: Tern This Statement Around and Go Home;

When looking for representative lines, ternaries are almost easy mode. While there’s nothing wrong with a good ternary expression, they have a bad reputation because they can quickly drift out towards “utterly unreadable”.

Or, sometimes, they can drift towards “incredibly stupid”. This anonymous submission is a pretty brazen example of the latter:

return (accounts == 1 ? 1 : accounts)

Presumably, once upon a time, this was a different expression. The code changed. Nobody thought about what was changing or why. They just changed it and moved on. Or, maybe, they did think about it, and thought, “someday this might go back to being complicated again, so I’ll leave the ternary in place”, which is arguably a worse approach.

We’ll never know which it was.

Since that was so simple, let’s look at something a little uglier, as a bonus. “WDPS” sends along a second ternary violation, this one has the added bonus of being in Objective-C. This code was written by a contractor (whitespace added to keep the article readable- original is all on one line):

    NSMutableArray *buttonItems = [NSMutableArray array];
    buttonItems = !negSpacer && !self.buttonCog
            ? @[] : (!negSpacer && self.buttonCog 
            ? @[self.buttonCog] : (!self.buttonCog && negSpacer 
            ? @[negSpacer] : @[negSpacer,self.buttonCog]));

This is a perfect example of a ternary which simply got out of control while someone tried to play code golf. Either this block adds no items to buttonItems, or it adds a buttonCog or it adds a negSpacer, or it adds both. Which means it could more simply be written as:

   NSMutableArray *buttonItems = [NSMutableArray array];
   if (negSpacer) {
        [buttonItems addObject:negSpacer];
    }
    if (self.buttonCog) {
        [buttonItems addObject:self.buttonCog];
    }
[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.


CodeSOD: Isn't There a Vaccine For MUMPS?;

Alex F is suffering from a disease. No, it’s not disfiguring, it’s not fatal. It’s something much worse than that.

It’s MUMPS.

MUMPS is a little bit infamous. MUMPS is its own WTF.

Alex is a support tech, which in their organization means that they sometimes write up tickets, or for simple problems even fix the code themselves. For this issue, Alex wrote up a ticket, explaining that the users was submitting a background job to run a report, but instead got an error.

Alex sent it to the developer, and the developer replied with a one line code fix:

 i $$zRunAsBkgUser(desc_$H,"runReportBkg^KHUTILLOCMAP",$na(%ZeData)) d
 . w !,"Search has been started in the background."
 e  w !,"Search failed to start in the background."

Alex tested it, and… it didn’t work. So, fully aware of the risks they were taking, Alex dug into the code, starting with the global function $$zRunAsBkgUser.

Before I post any more code, I am legally required to offer a content warning: the rest of this article is going to be full of MUMPS code. This is not for the faint of heart, and TDWTF accepts no responsibility for your mental health if you continue. Don’t read the rest of this article if you have eaten any solid food in the past twenty minutes. If you experience a rash, this may be a sign of a life threatening condition, and you should seek immediate treatment. Do not consume alcohol while reading this article. Save that for after, you’ll need it.

 ;---------
  ; NAME:         zRunAsBkgUser
  ; SCOPE:        PUBLIC
  ; DESCRIPTION:  Run the specified tag as the correct OS-level background user. The process will always start in the system default time zone.
  ; PARAMETERS:
  ;  %uJobID (I,REQ)      - Free text string uniquely identifying the request
  ;                         If null, the tag will be used instead but -- as this is not guaranteed unique -- this ID should be considered required
  ;  %uBkgTag (I,REQ)     - The tag to run
  ;  %uVarList (I,OPT)    - Variables to be passed from the current process' symbol table
  ;  %uJobParams (I,OPT)  - An array of additional parameters to be passed to %ZdUJOB
  ;                         Should be passed with the names of the parameters in %ZdUJOB, e.g. arr("%ZeDIR")="MGR"
  ;                         Currently supports only: %ZeDIR, %ZeNODE, %ZeBkOv
  ;  %uError (O,OPT)      - Error message in case of failure
  ;  %uForceBkg (I,OPT)   - If true, will force the request to be submitted to %ZeUMON
  ;  %uVerifyCond (I,OPT) - If null, this tag will return immediately after submitting the request
  ;                         If non-null, should contain code that will be evaluated to determine the success or failure of the request
  ;                         Will be executed as s @("result=("_%uVerifyCond_")")
  ;  %uVerifyTmo (I,OPT)  - Length of time, in seconds, to try to verify the success of the request
  ;                         Defaults to 1 second
  ; RETURNS:      If %uVerifyCond is not set: 1 if it's acceptable to run, 0 otherwise
  ;               If %uVerifyCond is set: 1 if the condition is verified after the specified timeout, 0 otherwise
zRunAsBkgUser(%uJobID,%uBkgTag,%uVarList,%uJobParams,%uError,%uForceBkg,%uVerifyCond,%uVerifyTmo) q $$RunBkgJob^%ZeUMON($$zCurrRou(),%uJobID,%uBkgTag,%uVarList,.%uJobParams,.%uError,%uForceBkg,%uVerifyCond,%uVerifyTmo) ;;#eof#  ;;#inline#

Thank the gods for comments, I guess. Alex’s eyes locked upon the sixth parameter- %uForceBkg. That seems a bit odd, for a function which is supposed to be submitting a background job. The zRunAsBkgUser function is otherwise quite short- it’s a wrapper around RunBkgJob.

Let’s just look at the comments:

 ;---------
  ; NAME:         RunBkgJob
  ; SCOPE:        INTERNAL
  ; DESCRIPTION:  Submit request to monitor daemon to run the specified tag as a background process
  ;               Used to ensure the correct OS-level user in the child process
  ;               Will fork off from the current process if the correct OS-level user is already specified,
  ;               unless the %uForceBkg flag is set. It will always start in the system default time zone.
  ; KEYWORDS:     run,background,job,submit,%ZeUMON,correct,user
  ; CALLED BY:    ($$)zRunAsBkgUser
  ; PARAMETERS:
  ;  %uOrigRou (I,REQ)    - The routine submitting the request
  ;  %uJobID (I,REQ)      - Free text string uniquely identifying the request
  ;                         If null, the tag will be used instead but -- as this is not guaranteed unique -- this ID should be considered required
  ;  %uBkgTag (I,REQ)     - The tag to run
  ;  %uVarList (I,OPT)    - Variables to be passed from the current process' symbol table
  ;                         If "", pass nothing; if 1, pass everything
  ;  %uJobParams (I,OPT)  - An array of additional parameters to be passed to %ZdUJOB
  ;                         Should be passed with the names of the parameters in %ZdUJOB, e.g. arr("%ZeDIR")="MGR"
  ;                         Currently supports only: %ZeDIR, %ZeNODE, %ZeBkOv
  ;  %uError (O,OPT)      - Error message in case of failure
  ;  %uForceBkg (I,OPT)   - If true, will force the request to be submitted to %ZeUMON
  ;  %uVerifyCond (I,OPT) - If null, this tag will return immediately after submitting the request
  ;                         If non-null, should contain code that will be evaluated to determine the success or failure of the request
  ;                         Will be executed as s @("result=("_%uVerifyCond_")")
  ;  %uVerifyTmo (I,OPT)  - Length of time, in seconds, to try to verify the success of the request
  ;                         Defaults to 1 second
  ; RETURNS:      If %uVerifyCond is not set: 1 if it's acceptable to run, 0 otherwise
  ;               If %uVerifyCond is set: 1 if the condition is verified after the specified timeout, 0 otherwise

Once again, the suspicious uForceBkg parameter is getting passed it. The comments claim that this only controls the timezone, which implies either the parameter is horribly misnamed, or the comments are wrong. Or, possibly, both. Wait, no, it's talking about ZeUMON. My brain wants it to be timezones. MUMPS is getting to me. Since the zRunAsBkgUser has different comments, I suspect it’s both, but it’s MUMPS. I have no idea what could happen. Let’s look at the code.

  RunBkgJob(%uOrigRou,%uJobID,%uBkgTag,%uVarList,%uJobParams,%uError,%uForceBkg,%uVerifyCond,%uVerifyTmo) ;
  n %uSecCount,%uIsStarted,%uCondCode,%uVarCnt,%uVar,%uRet,%uTempFeat
  k %uError
  i %uBkgTag="" s %uError="Need to pass a tag" q 0
  i '$$validrou(%uBkgTag) s %uError="Tag does not exist" q 0
  ;if we're already the right user, just fork off directly
  i '%uForceBkg,$$zValidBkgOSUser() d  q %uRet
  . d inheritOff^%ZdDEBUG()
  . s %uRet=$$^%ZdUJOB(%uBkgTag,"",%uVarList,%uJobParams("%ZeDIR"),%uJobParams("%ZeNODE"),$$zTZSystem(1),"","","","",%uJobParams("%ZeOvBk"))
  . d inheritOn^%ZdDEBUG()
  ;
  s:%uJobID="" %uJobID=%uBkgTag   ;this *should* be uniquely identifying, though it might not be...
  s ^%ZeUMON("START","J",%uJobID,"TAG")=%uBkgTag
  s ^%ZeUMON("START","J",%uJobID,"CALLER")=%uOrigRou
  i $$zFeatureCanUseTempFeatGlobal() s %uTempFeat=$$zFeatureSerializeTempGlo() s:%uTempFeat'="" ^%ZeUMON("START","J",%uJobID,"FEAT")=%uTempFeat
  m:$D(%uJobParams) ^%ZeUMON("START","J",%uJobID,"PARAMS")=%uJobParams
  i %uVarList]"" d
  . s ^%ZeUMON("START","J",%uJobID,"VARS")=%uVarList
  . d inheritOff^%ZdDEBUG()
  . i %uVarList=1 d %zSavVbl($name(^%ZeUMON("START","J",%uJobID,"VARS"))) i 1   ;Save whole symbol table if %uVarList is 1
  . e  f %uVarCnt=1:1:$L(%uVarList,",") s %uVar=$p(%uVarList,",",%uVarCnt) m:%uVar]"" ^%ZeUMON("START","J",%uJobID,"VARS",%uVar)=@%uVar
  . d inheritOn^%ZdDEBUG()
  s ^%ZeUMON("START","G",%uJobID)=""   ;avoid race conditions by setting pointer only after the data is complete
  d log("BKG","Request to launch tag "_%uBkgTag_" from "_%uOrigRou)
  q:%uVerifyCond="" 1   ;don't hang around if there's no need
  d
  . s %uError="Verification tag crashed"
  . d SetTrap^%ZeERRTRAP("","","Error verifying launch of background tag "_%uBkgTag)
  . s:%uVerifyTmo<1 %uVerifyTmo=1
  . s %uIsStarted=0
  . s %uCondCode="%uIsStarted=("_%uVerifyCond_")"
  . f %uSecCount=1:1:%uVerifyTmo h 1 s @%uCondCode q:%uIsStarted
  . d ClearTrap^%ZeERRTRAP
  . k %uError
  i %uError="",'%uIsStarted s %uError="Could not verify that job started successfully"
  q %uIsStarted
  ;
  q  ;;#eor#

Well, there you have it, the bug is so simple to spot, I’ll leave it as an exercise to the readers.

I’m kidding. The smoking gun, as Alex calls it, is the block:

  i '%uForceBkg,$$zValidBkgOSUser() d  q %uRet
  . d inheritOff^%ZdDEBUG()
  . s %uRet=$$^%ZdUJOB(%uBkgTag,"",%uVarList,%uJobParams("%ZeDIR"),%uJobParams("%ZeNODE"),$$zTZSystem(1),"","","","",%uJobParams("%ZeOvBk"))
  . d inheritOn^%ZdDEBUG()
  ;

This is what passes for an “if” statement in MUMPS. Specifically, if the %uForceBkg parameter is set, and the zValidBkgOSUser function returns true, then we’ll submit the job. Otherwise, we don’t submit the job, and thus get errors when we check on whether or not it’s done.

So, the underlying bug, such as it were, is a confusing parameter with an unreasonable default. This is not all that much of a WTF, I admit, but I really really wanted you all to see this much MUMPS code in a single sitting, and I wanted to remind you: there are people who work with this every day.

[Advertisement] Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!


A Shell Game;

When the big banks and brokerages on Wall Street first got the idea that UNIX systems could replace mainframes, one of them decided to take the plunge - Big Bang style. They had hundreds of programmers cranking out as much of the mainframe functionality as they could. Copy-paste was all the rage; anything to save time. It could be fixed later.

Nyst 1878 - Cerastoderma parkinsoni R-klep

Senior management decreed that the plan was to get all the software as ready as it could be by the deadline, then turn off and remove the mainframe terminals on Friday night, swap in the pre-configured UNIX boxes over the weekend, and turn it all on for Monday morning. Everyone was to be there 24 hours a day from Friday forward, for as long as it took. Air mattresses, munchies, etc. were brought in for when people would inevitably need to crash.

While the first few hours were rough, the plan worked. Come Monday, all hands were in place on the production floor and whatever didn't work caused a flurry of activity to get the issue fixed in very short order. All bureaucracy was abandoned in favor of: everyone has root in order to do whatever it takes on-the-fly, no approvals required. Business was conducted. There was a huge sigh of relief.

Then began the inevitable onslaught of add this and that for all the features that couldn't be implemented by the hard cutoff. This went on for 3-4 years until the software was relatively complete, but in desperate need of a full rewrite. The tech people reminded management of their warning about all the shortcuts to save time up front, and that it was time to pay the bill.

To their credit, management gave them the time and money to do it. Unfortunately, copy-paste was still ingrained in the culture, so nine different trading systems had about 90% of their code identical to their peers, but all in separate repositories, each with slightly different modification histories to the core code.

It was about this time that I joined one of the teams. The first thing they had me do was learn how to verify that all 87 (yes, eighty seven) of the nightly batch jobs had completed correctly. For this task, both the team manager and lead dev worked non-stop from 6AM to 10AM - every single day - to verify the results of the nightly jobs. I made a list of all of the jobs to check, and what to verify for each job. It took me from 6AM to 3:00PM, which was kind of pointless as the markets close at 4PM.

After doing it for one day, I said no way and asked them to continue doing it so as to give me time to automate it. They graciously agreed.

It took a while, but I wound up with a rude-n-crude 5K LOC ksh script that reduced the task to checking a text file for a list of OK/NG statuses. But this still didn't help if something had failed. I kept scripting more sub-checks for each task to implement what to do on failure (look up what document had the name of the job to run, figure out what arguments to pass, etc., get the status of the fix-it job, and notify someone on the upstream system if it still failed, etc). Either way, the result was recorded.

In the end, the ksh script had grown to more than 15K LOC, but it reduced the entire 8+ hour task to checking a 20 digit (bit-mask) page once a day. Some jobs failed every day for known reasons, but that was OK. As long as the bit-mask of the page was the expected value, you could ignore it; you only had to get involved if an automated repair of something was attempted but failed (this only happened about once every six months).

In retrospect, there were better ways to write that shell script, but it worked. Not only did all that nightly batch job validation and repair logic get encoded in the script (with lots of documentation of the what/how/why variety), but having rid ourselves of the need to deal with this daily mess freed up one man-day per day, and more importantly, allowed my boss to sleep later.

One day, my boss was bragging to the managers of the other trading systems (that were 90% copy-pasted) that he no longer had to deal with this issue. Since they were still dealing with the daily batch-check, they wanted my script. Helping peer teams was considered a Good Thing™, so we gave them the script and showed them how it worked, along with a detailed list of things to change so that it would work with the specifics of their individual systems.

About a week later, the support people on my team (including my boss) started getting nine different status pages in the morning - within seconds of each other - all with different status codes.

It turns out the other teams only modified the program and data file paths for the monitored batch jobs that were relevant to their teams, but didn't bother to delete the sections for the batch jobs they didn't need, and didn't update the notification pager list with info for their own teams. Not only did we get the pages for all of them, but this happened on the one day in six months that something in our system really broke and required manual intervention. Unfortunately, all of the shell scripts attempted to auto correct our failed job. Without. Any. Synchronization. By the time we cleared the confusion of the multiple pages, figured out the status of our own system, realized something required manual fixing and started to fix the mess created by the multiple parallel repair attempts, there wasn't enough time to get it running before the start of business. The financial users were not amused that they couldn't conduct business for several hours.

Once everyone changed the notification lists and deleted all the sections that didn't apply to their specific systems, the problems ceased and those batch-check scripts ran daily until the systems they monitored were finally retired.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!


A Tapestry of Threads;

A project is planned. Gantt charts are drawn up. Timelines are set. They're tight up against the critical path, because including any slack time in the project plan is like planning for failure. PMs have meetings. Timelines slip. Something must be done, and the PMs form a nugget of a plan.

CORINTI

That nugget squeezes out of their meeting, and rolls downhill until it lands on some poor developer's desk.

That poor developer was Alona.

"Rex is the lead architect on this," her manager said. "And the project is about 90% complete… but even with that, they're never going to hit their timeline. So we're going to kinda do an 'all hands' thing to make sure that the project completes on time."

Alona was a junior developer, but even with that, she'd seen enough projects to know: slamming new resources onto a project in its final days never speeds up its completion. Even so, she had her orders.

Alona grabbed the code, checked the project backlog, confirmed her build environment, and then talked to the other developers. They had some warnings.

"It uses a lot of threads, and… well, the thread model is kinda weird, but Rex says it's the right way to do this."

"I don't understand what's going on with these threads, but I'm sure Rex could explain it to you."

"It's heavily CPU bound, but we're using more threads than we have cores, but Rex says we have to do it that way in order to get the performance we need."

Alona had never met Rex, but none of that sounded good. The first tasks Alona needed to grab off the backlog didn't have anything to do with the threads, so she spent a few days just writing code, until she picked up a bug which was obviously caused by a race condition.

From what she'd seen in the documentation and the code, that meant the problem had to be somewhere in MainComputationThread. She wasn't sure where it was defined, so she just did a quick search for the term class MainComputationThread.

It returned twenty hits. There was no class called MainComputationThread, though there was an interface. There were also classes which implemented that interface, named things like MainComputationThread1 and MainComputationThread17. A quick diff showed that all twenty of the MainComputationThreadn classes were 1,243 lines of perfectly identical code.

They were also all implemented as singletons.

Alona had never met Rex, and didn't want to, but she needed to send him an email and ask: "Why?"

Threading is a pretty advanced programming topic, so I put together an easy to use framework for writing threaded code, so that junior developers can use it. Just use it. -Rex

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!


Error'd: Truth in Errors;

Jakub writes, "I'm not sure restarting will make IE 'normal', but yeah, I guess it's worth a shot."

 

"What else can I say? Honest features are honest," wrote Matt H.

 

"This was the sign outside the speaker ready-room for the duration of American Mensa's 2018 Annual Gathering at the JW Marriott in Indianapolis," Dave A., "Of course, we weren't in control of the signs...or the fans...or the fan speeds."

 

"Well, Cisco made an attempt to personalize this email," wrote Pascal.

 

Dave A. wrote, "Skynet is coming (to the Chubby Squirrel Brewery, opened that very day), but we can do anything we want with it because there are no Term(s) of Use."

 

"Yeah, GMail, I agree, there's no way that number of fireworks all at once is safe," Ranuka wrote.

 

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!


CodeSOD: Knowledge Transfer;

Lucio Crusca is a consultant with a nice little portfolio of customers he works with. One of those customers was also a consultancy, and their end customer had a problem. The end customer's only in-house developer, Tyrell, was leaving. He’d worked there for 8 years, and nobody else knew anything about his job, his code, or really what exactly he’d been doing for 8 years.

They had two weeks to do a knowledge transfer before Tyrell was out the door. There was no chance of on-boarding someone in that time, so they wanted a consultant who could essentially act as a walking, talking USB drive, simply holding all of Tyrell’s knowledge until they could have a full-time developer.

As you can imagine, the two week brain-dump turned into a two week “documentation crunch” as pretty much nothing had any real documentation. That lead to comments like:

  /**
   * Parses a log file. This function looks for the string "ERR-001" in the log file
   * produced by the parser daemon (see project NetParserDaemon).
   * If found it returns the file contents with the string. Otherwise it
   * returns the file contents without it. 
   * Known bug: it needs more optimizations to handle very big files. In the 
   * meantime we manually restart the parser daemon from time to time, so the
   * log file doesn't grow too much.
   * @param log_file_name File name
   * @return ArrayList
   */
  public static ArrayList parseLogFile(String log_file_name) {
    …
  }

Read that comment, read the signature, and tell me if you have any idea what that method does? Because trust me, after reading the implementation, it’s not going to get any clearer.

  public static ArrayList parseLogFile(String log_file_name)
  {
    try
    {
      ArrayList result = new ArrayList();
      File f = new File(log_file_name);
      FileInputStream s = new FileInputStream(log_file_name);
      InputStreamReader r = new InputStreamReader(s);
      BufferedReader r2 = new BufferedReader(r);
      String line = null;
      int retry = 1;
      while (r2.readLine() != null)
      {
        try
        {
          for (int i = 0; i < retry; i++)
          {
            line = r2.readLine();
            result.add(line);
          }
          if (line.contains("ERR-001"))
            return result;
        }
        catch (OutOfMemoryError e)
        {
          System.gc();
          line = "Retry";
          retry = (int)(Math.random() * 10 + 10);
        }
      }
    }
    catch(Exception e)
    {
      ArrayList result = new ArrayList();
      result.add("ERR-001: File not found");
      return result;
    }
    finally
    {
      // always return the file contents, even when there are exceptions
      return parseLogFile(log_file_name);
    }
  }

There’s just… so much going on here. First, this method must be dead code. The return in the finally block always trumps any other return in Java, which means it would call itself recursively until a stack overflow. Also, don’t put returns in your finally block.

So it doesn’t work, clearly hasn’t been tested, and certainly can’t be invoked.

But the core loop demonstrates its own bizarre logic. We retry reading within a for loop, by default though, we only do 1 retry. If and only if we encounter an out of memory exception, then we set the retry to a random value and repeat the loop. Oh, and don’t forget to garbage collect first. If any other exception happens, we’ll catch that and just return an ArrayList with “ERR–001: File not found” in it, which raises some questions about what on earth ERR-001 actually means to this application.

By the time the company actually hired on a full-time developer, Lucio had already forgotten most of the knowledge dump, and the rushed documentation and broken code meant that there really wasn’t much knowledge to transfer to the new developer, beyond, “Delete it, destroy the backups, and start from scratch.”

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!