6 ms·

It's a nice write up, but the "Lessons learned" is terrible, it appears as if they didn't learn anything and view the whole thing as an infrastructure problem.

It's a nice write up, but the "Lessons learned" is terrible, it appears as if they didn't learn anything and view the whole thing as an infrastructure problem. As others pointed out, why does Firefox even need to communicate with Mozilla services? Sure, telemetry needs to feed data back, if enabled, but if that fails why does it need to stop the browser from working? Shouldn't the lesson learned be: The telemetry functionality in Firefox has a bug, where an infrastructure outage at Mozilla can "break" the browser. The fix isn't in infrastructure, the fix has to be in the code that communicates back, it should fail gracefully. It's not a problem if telemetry fail, either cache locally and just drop the data, it's honestly not important. I'm sorry, I get that it's interesting how and why all this failed, but Mozilla makes it seem like they don't get what the root of the problem is.

floatingatoll 5y ago
Per other comments here, supposedly the same issue would have occurred with a variety of non-telemetry tasks as well. One of them is indicated to be certificate-related, which suggests to me that CRL lookups could have triggered it, though I don’t know for sure. It ended up being that the issue occurred first with telemetry rather than with certificate stuff or who knows what else. But the flaw wasn’t in telemetry code, so focusing on telemetry wouldn’t have prevented this at all.
progbits 5y ago
Exactly. I love Firefox and use it as my primary browser but this is the wrong conclusion. Don't make your telemetry backend more reliable. Instead break it on purpose several times a day. That way a similar bug in the browser will not make it past dev channel.
- Yoric 5y ago
  As mentioned in a few places, the telemetry backend was not the culprit. The network stack was. The network stack is already pretty heavily tested and fuzzed, but obviously that didn't catch this specific bug.
  - cxr 5y ago
    How is this a response to the message you're replying to? You seem not to have understood it at all.
    - deleted 5y ago
      [deleted]
  - miyuru 5y ago
    I also get that telemetry was not main culprit, but the way they are have ordered the "lessons learned" list they seem to be blaming GCP which THEY use (not end users) for telemetry collection. so they are unknowingly acknowledging the telemetry collection a major issue on the incident. My thinking is that list should be reordered as ultimate culprit to blame is firefox itself.(make the third point on lessons learned as the first one)
tjoff 5y ago
Not sure how you reach that conclusion, the root cause is described in detail. Of course they'll fix so that this problem doesn't occur in the future. But as said, it had nothing to do with telemetry. Just that telemetry happened to trigger the bug.
- mrweasel 5y ago
  That's fair, I just got the impressing that they where more focused on the infrastructure aspects of the problem, rather than the bugs in the Necko and Viaduct code.
smorgusofborg 5y ago
I was also disappointed for these reasons but couldn't have stated it this well. I also find it alarming in this age of citizen hostile countries blocking/rerouting Internet and J-curve countries having intentional calamities to think that everyone who uses Firefox might hit bizarre technical failure contacting unnecessary centralized servers like this as a first barrier to get past before following common DNS setting instructions to get to whatever local networking people are assembling. (That's a lot more graffiti than "8.8.8.8".)
- tjoff 5y ago
  If you came to that conclusion then you have not understood the post.
  - smorgusofborg 5y ago
    If I have not understood the post then it is probably their communication style. They lay out a bunch of things to fix about a system that could bring down your browser. What if that system that communication with Google about GCP messed up for them isn't even the IPs you are contacting during unrest in Khazakhstan or an election in Uganda? What if it is a semi-intentionally confused transparent proxy? Testing a few more things that a friendly proxy may do as it improves your connection is hardly the same as assuming the worst about your network in proper paranoia mode.
    - tjoff 5y ago
      They lay out one thing that can bring down your browser if a bunch of circumstances are true. They are fixing that thing and all those circumstances and try to make sure that circumstances of that characteristic won't happen again. The what-ifs you are talking about could just as well be any homepage on the internet. ... and that page could also be MITMed. So your point is to not have bugs?
      - smorgusofborg 5y ago
        You expect bugs, I expect bugs, they expect bugs.. This explains why they lead with a discussion of defenses they were taking like certificate pinning or encrypting this tracking that caused them to discount risks of putting this service in a new system and letting it run on startup and contact a 3rd party? Except that isn't what they lead with, they have done nothing to reiterate a position that's appropriate for a browser maker.
mozdeco 5y ago
> the fix has to be in the code that communicates back, it should fail gracefully. The bug that caused the hang was in the network stack itself. There was no way the calling code could have prevented this in any way. You can see this by taking a look at the linked HTTP3 code. It's not that the higher-level code kept retrying over and over causing the hang, that was not the problem here. Under "Lessons learned" you can also read "investigating action points both to make the browser more resilient towards such problems". I agree that this is broadly spoken, but it covers ideas that would have made this technically recoverable (e.g. can network requests be compartmentalized to not block on a single network thread?).
- account42 5y ago
  > There was no way the calling code could have prevented this in any way. It could have prevented it by not making the call in the first place.
  - acdha 5y ago
    As explained in the article, this problem was not specific to Telemetry: “This is why users who disabled Telemetry would see this problem resolved even though the problem is not related to Telemetry functionality itself and could have been triggered otherwise.” Since a browser's job is to make HTTP requests, a bug in the network stack would almost certainly have been hit in other places. This was highly-visible so it was quickly noticed but it's quite possible that a less frequent trigger could have plagued Firefox users for a much longer period of time as HTTP/3 adoption increases.
    - account42 5y ago
      The article specifically states that normal web requests went through a different code path that did not trigger the bug. That the bug was not technically in the telemetry code is irrelevant - it happened without user interaction because of telemetry and it did not happen (at least as often) with telemetry disabled. Saying that there was no way to prevent it assumes that telemetry could not have been disabled/removed, which is false.
scratcheee 5y ago
They explained that in the retrospective. The code _does_ work the way you describe, _except_ for the latent bug that caused the networking thread to get stuck in an infinite loop, which it was never supposed to do, even when errors occur. It was never supposed to work that way, and the fact it did was because of a bug they'd never seen before. So it wasn't that "oops, we shouldn't have built the system to get stuck forever when it fails" but rather "this bug triggered that bug which combined to cause a far worse result than 1 bug alone could have". The only "lesson learnt" there is either that they need better ways to find bugs, quadruple up their thread count just so that different subsystems can't coexist on the same threads to avoid a theoretical problem that shouldn't ever happen again, or they just come up with infrastructural changes to minimise the negative results of the next "2 bugs reacted together and caught fire" scenario, which is the one they went with, and the only sane one.
- account42 5y ago
  Or they could reduce complexity by not adding things like telemetry, which has no direct user benefit and therefore should not be included in release versions. There should be no service that all firefox installs connect to.
  - Gigachad 5y ago
    Telemetry has massive direct user benefit. It gives every user a vote on how important each feature is rather than letting power users who manually submit feedback control the show.
    - account42 5y ago
      Even ignoring that decisions based on Telemetry are very much influenced by the person making the decision, importance has nothing to do with how much a feature is used.
    - gilrain 5y ago
      Incorrect. It gives the designers and developers an opaque dataset of user behavior which they can interpret in many ways. I find their interpretations of this data to be highly motivated and suspect.
deleted 5y ago
[deleted]
iakov 5y ago
After reading this I, Firefox user of many years, learned my lesson and downloaded Brave. I hope that Brave will be a better user agent than FF.
jillesvangurp 5y ago
Well the telemetry is something that is separate topic. But regardless, ny code that can end up blocking forever should have a timeout and recover from that timeout happening. The problem here was that something that is known to fail for all sorts of reasons (network IO) was happening without such a timeout. Or with a timeout with a failure mode that it never happens (yikes). That's a design problem and even something with a very small chance of happening is extremely likely to actually happen at some point with a product that is this widely used. This stuff is hard of course and I end up addressing issues related to his once in a while. The fix is usually to surround such code with defensive measures such as timeouts, retry mechanisms, telemetry, logging, etc. The additional question/learning is why they never noticed this happening before. Because it probably did; they just never noticed because the very thing that would have told them was actually hanging. People killing an application for whatever reason is something that you'd want to know however.
- mozdeco 5y ago
  > code that can end up blocking forever should have a timeout and recover from that timeout happening. There was no way for the calling code to do this. This was literally an infinite loop inside the network stack. Imagine the network stack itself going `while(1) {}` on you, without checking if the request was canceled. Even if you detect that this happens, there is nothing you can do as the caller. You can't even properly stop the thread, as it is not cooperating. So recovering from this type of failure is hard.
  - throwaway984393 5y ago
    I went and looked at the code (it's linked in the article). You absolutely can put a timeout around a case/switch statement. There's like 5 different ways to do it. And the code calling network syscalls can also have timeouts, obviously; otherwise nobody would ever be able to time out any blocked network operation. This is all network programming 101.
    - acdha 5y ago
      If it's that easy, I'm sure they'd accept your pull request.
jrochkind1 5y ago
It was intended to fail gracefully. It was a bug. They did fix the bug, involving a complicated interaction between different parts of the network stack. But "we should try harder not to have bugs" is, rightfully, seldom considered a valuable "lesson learned".
rowls66 5y ago
I agree that the lessons learned section is pretty weak. I think that the root cause of the incident was poor code quality, and in particular, using a weekly typed data structure (String) to pass data between system modules that each interpreted that data differently. The suggested learning was that more testing should have been done, but as a solution, more testing is a cop out. A real solution is to develop code in a language that supports a robust type system, and then using that type system effectively in development. Not an easy solution, so in the short term, we'll have more testing, and more bugs.

floatingatoll 5y ago

Per other comments here, supposedly the same issue would have occurred with a variety of non-telemetry tasks as well. One of them is indicated to be certificate-related, which suggests to me that CRL lookups could have triggered it, though I don’t know for sure. It ended up being that the issue occurred first with telemetry rather than with certificate stuff or who knows what else. But the flaw wasn’t in telemetry code, so focusing on telemetry wouldn’t have prevented this at all.

progbits 5y ago

Exactly. I love Firefox and use it as my primary browser but this is the wrong conclusion. Don't make your telemetry backend more reliable. Instead break it on purpose several times a day. That way a similar bug in the browser will not make it past dev channel.

Yoric 5y ago
As mentioned in a few places, the telemetry backend was not the culprit. The network stack was. The network stack is already pretty heavily tested and fuzzed, but obviously that didn't catch this specific bug.
- cxr 5y ago
  How is this a response to the message you're replying to? You seem not to have understood it at all.
  - deleted 5y ago
    [deleted]
- miyuru 5y ago
  I also get that telemetry was not main culprit, but the way they are have ordered the "lessons learned" list they seem to be blaming GCP which THEY use (not end users) for telemetry collection. so they are unknowingly acknowledging the telemetry collection a major issue on the incident. My thinking is that list should be reordered as ultimate culprit to blame is firefox itself.(make the third point on lessons learned as the first one)

tjoff 5y ago

Not sure how you reach that conclusion, the root cause is described in detail. Of course they'll fix so that this problem doesn't occur in the future. But as said, it had nothing to do with telemetry. Just that telemetry happened to trigger the bug.

mrweasel 5y ago
That's fair, I just got the impressing that they where more focused on the infrastructure aspects of the problem, rather than the bugs in the Necko and Viaduct code.

smorgusofborg 5y ago

I was also disappointed for these reasons but couldn't have stated it this well. I also find it alarming in this age of citizen hostile countries blocking/rerouting Internet and J-curve countries having intentional calamities to think that everyone who uses Firefox might hit bizarre technical failure contacting unnecessary centralized servers like this as a first barrier to get past before following common DNS setting instructions to get to whatever local networking people are assembling. (That's a lot more graffiti than "8.8.8.8".)

tjoff 5y ago
If you came to that conclusion then you have not understood the post.
- smorgusofborg 5y ago
  If I have not understood the post then it is probably their communication style. They lay out a bunch of things to fix about a system that could bring down your browser. What if that system that communication with Google about GCP messed up for them isn't even the IPs you are contacting during unrest in Khazakhstan or an election in Uganda? What if it is a semi-intentionally confused transparent proxy? Testing a few more things that a friendly proxy may do as it improves your connection is hardly the same as assuming the worst about your network in proper paranoia mode.
  - tjoff 5y ago
    They lay out one thing that can bring down your browser if a bunch of circumstances are true. They are fixing that thing and all those circumstances and try to make sure that circumstances of that characteristic won't happen again. The what-ifs you are talking about could just as well be any homepage on the internet. ... and that page could also be MITMed. So your point is to not have bugs?
    - smorgusofborg 5y ago
      You expect bugs, I expect bugs, they expect bugs.. This explains why they lead with a discussion of defenses they were taking like certificate pinning or encrypting this tracking that caused them to discount risks of putting this service in a new system and letting it run on startup and contact a 3rd party? Except that isn't what they lead with, they have done nothing to reiterate a position that's appropriate for a browser maker.

mozdeco 5y ago

> the fix has to be in the code that communicates back, it should fail gracefully. The bug that caused the hang was in the network stack itself. There was no way the calling code could have prevented this in any way. You can see this by taking a look at the linked HTTP3 code. It's not that the higher-level code kept retrying over and over causing the hang, that was not the problem here. Under "Lessons learned" you can also read "investigating action points both to make the browser more resilient towards such problems". I agree that this is broadly spoken, but it covers ideas that would have made this technically recoverable (e.g. can network requests be compartmentalized to not block on a single network thread?).

account42 5y ago
> There was no way the calling code could have prevented this in any way. It could have prevented it by not making the call in the first place.
- acdha 5y ago
  As explained in the article, this problem was not specific to Telemetry: “This is why users who disabled Telemetry would see this problem resolved even though the problem is not related to Telemetry functionality itself and could have been triggered otherwise.” Since a browser's job is to make HTTP requests, a bug in the network stack would almost certainly have been hit in other places. This was highly-visible so it was quickly noticed but it's quite possible that a less frequent trigger could have plagued Firefox users for a much longer period of time as HTTP/3 adoption increases.
  - account42 5y ago
    The article specifically states that normal web requests went through a different code path that did not trigger the bug. That the bug was not technically in the telemetry code is irrelevant - it happened without user interaction because of telemetry and it did not happen (at least as often) with telemetry disabled. Saying that there was no way to prevent it assumes that telemetry could not have been disabled/removed, which is false.

scratcheee 5y ago

They explained that in the retrospective. The code _does_ work the way you describe, _except_ for the latent bug that caused the networking thread to get stuck in an infinite loop, which it was never supposed to do, even when errors occur. It was never supposed to work that way, and the fact it did was because of a bug they'd never seen before. So it wasn't that "oops, we shouldn't have built the system to get stuck forever when it fails" but rather "this bug triggered that bug which combined to cause a far worse result than 1 bug alone could have". The only "lesson learnt" there is either that they need better ways to find bugs, quadruple up their thread count just so that different subsystems can't coexist on the same threads to avoid a theoretical problem that shouldn't ever happen again, or they just come up with infrastructural changes to minimise the negative results of the next "2 bugs reacted together and caught fire" scenario, which is the one they went with, and the only sane one.

account42 5y ago
Or they could reduce complexity by not adding things like telemetry, which has no direct user benefit and therefore should not be included in release versions. There should be no service that all firefox installs connect to.
- Gigachad 5y ago
  Telemetry has massive direct user benefit. It gives every user a vote on how important each feature is rather than letting power users who manually submit feedback control the show.
  - account42 5y ago
    Even ignoring that decisions based on Telemetry are very much influenced by the person making the decision, importance has nothing to do with how much a feature is used.
  - gilrain 5y ago
    Incorrect. It gives the designers and developers an opaque dataset of user behavior which they can interpret in many ways. I find their interpretations of this data to be highly motivated and suspect.

deleted 5y ago

[deleted]

iakov 5y ago

After reading this I, Firefox user of many years, learned my lesson and downloaded Brave. I hope that Brave will be a better user agent than FF.

jillesvangurp 5y ago

Well the telemetry is something that is separate topic. But regardless, ny code that can end up blocking forever should have a timeout and recover from that timeout happening. The problem here was that something that is known to fail for all sorts of reasons (network IO) was happening without such a timeout. Or with a timeout with a failure mode that it never happens (yikes). That's a design problem and even something with a very small chance of happening is extremely likely to actually happen at some point with a product that is this widely used. This stuff is hard of course and I end up addressing issues related to his once in a while. The fix is usually to surround such code with defensive measures such as timeouts, retry mechanisms, telemetry, logging, etc. The additional question/learning is why they never noticed this happening before. Because it probably did; they just never noticed because the very thing that would have told them was actually hanging. People killing an application for whatever reason is something that you'd want to know however.

mozdeco 5y ago
> code that can end up blocking forever should have a timeout and recover from that timeout happening. There was no way for the calling code to do this. This was literally an infinite loop inside the network stack. Imagine the network stack itself going `while(1) {}` on you, without checking if the request was canceled. Even if you detect that this happens, there is nothing you can do as the caller. You can't even properly stop the thread, as it is not cooperating. So recovering from this type of failure is hard.
- throwaway984393 5y ago
  I went and looked at the code (it's linked in the article). You absolutely can put a timeout around a case/switch statement. There's like 5 different ways to do it. And the code calling network syscalls can also have timeouts, obviously; otherwise nobody would ever be able to time out any blocked network operation. This is all network programming 101.
  - acdha 5y ago
    If it's that easy, I'm sure they'd accept your pull request.

jrochkind1 5y ago

It was intended to fail gracefully. It was a bug. They did fix the bug, involving a complicated interaction between different parts of the network stack. But "we should try harder not to have bugs" is, rightfully, seldom considered a valuable "lesson learned".

rowls66 5y ago

I agree that the lessons learned section is pretty weak. I think that the root cause of the incident was poor code quality, and in particular, using a weekly typed data structure (String) to pass data between system modules that each interpreted that data differently. The suggested learning was that more testing should have been done, but as a solution, more testing is a cop out. A real solution is to develop code in a language that supports a robust type system, and then using that type system effectively in development. Not an easy solution, so in the short term, we'll have more testing, and more bugs.