“My program needs to send some emails,” the developer thought to themselves. The then thought about how this could go wrong, probably thinking that, boy, they’ve got an external server, and boy, spam is a problem, and that server might get upset if you’re sending too many messages at once, or too quickly. That’s a thing which could happen, right?

That’s at least a plausible line of thought for the code Juana found, which looks something like this:

		public function processQueue($method)
		{
			$counter = 0;
			while (($result_item = $this->_getProcessableItem($method)) !== null) {

				// .....
				// Here we process the item, e.g. sending some email.
				// .....
                        
				if (++$counter % 10 == 0)
					sleep(2);
				else if (++$counter > 100)
					break;
			}
		}

So, this method grabs items out of a _getProcessableItem method, and as long as there are items in the queue to process, it does something to them, including sending an email. And then there are the if statements.

If you’re just skimming the code, it’s easy to miss the error. We check ++$counter to see if it’s divisible by ten, and if it is, sleep for two seconds. Why two? Why sleep at all? That information isn’t here, either in comment or commit message form. We just do.

Or do we? Because we also check if ++$counter > 100… which also increments the counter. That check works, but because it’s there, our mod–10 check doesn’t. Counting is hard, and since this loop essentially counts by two, ++$counter % 10 is always going to fail.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.