commit 54dfdec9606185fdffaec5aa85eea262bb375097
author Alice Kober-Sotzek <aliceks@google.com> Fri Apr 13 11:16:29 2018 +0200
committer Dave Borowitz <dborowitz@google.com> Fri Apr 13 05:43:41 2018 -0400
tree 8fdf23ca196179bc4846b087e7d9c8424bdee851
parent 5738641811de942269c18fa9db37d226545ddf1e

Fix infinite loop in IntraLineLoader

If edits due to rebase are present, IntraLineLoader#combineLineEdits
will enter an infinite loop. As we query the intraline diff cache with a
timeout check and use a fall-back behavior in case of a timeout, none of
our threads got completely stuck due to that bug. On the one hand, this
meant that the Gerrit server could still serve requests. On the other
hand, it concealed that bug and resulted in high CPU load, which could
very negatively impact the whole Gerrit server.

The fix of the issue is quite simple: We increase the index counter
appropriately. As this issue shows, the implementation in
IntraLineLoader#combineLineEdits is quite fragile. There are better ways
to achieve the same computation. Adjusting that will be left for the
future.

None of our tests caught that issue as the result of the fall-back
mechanism was the same as the result of the real computation for all
tests which were affected. For that reason, we add a test whose results
differ between the regular and the fall-back mechanism. In addition, we
add some tests which cover intraline edits directly. Up to now, we
didn't have any tests for them. We could and should certainly add more
in the future.

Ideally, we would also use a timeout for all tests within
RevisionDiffIT. Normally, this would be simply achieved by using the
Timeout rule of JUnit. However, adding that rule breaks all tests within
that class as our implementation of ConfigSuite doesn't seem to nicely
interact with arbitrary JUnit rules.

As the result of timed out intraline computations are cached, we need to
invalidate the intraline diff cache. The high CPU load also caused a lot
of PatchList cache computations to time out, for which also the
fall-back result was cached. Hence, we invalidate that cache as well.

Bug: Issue 8742
Change-Id: I0abf392d398320964a021099d72a4f876a73b6a6