Fix: OOM didn't force Teku to quit

[zilm13]

PR Description

Fixed Issue(s)

partially addresses #11225

Documentation

• I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

• I thought about adding a changelog entry, and added one if I deemed necessary.

---

Note

Medium Risk
Changes global process termination and async error handling; incorrect detection could exit on non-OOM errors or miss edge cases, though behavior is heavily tested and shutdown is idempotent.

Overview
Fixes a case where Teku could keep running after heap exhaustion because OutOfMemoryError was swallowed by async futures, suppressed subscriber/REST callbacks, or only detected via a direct instanceof check.

Adds FatalErrorHandler, which treats OOM (including wrapped causes and suppressed errors) as fatal, triggers shutdown once, and exits via a non-blocking path: graceful System.exit on a daemon thread plus a 90s watchdog that Runtime.halts if shutdown hooks wedge (e.g. stuck Jetty teardown).

Wires that handler into SafeFuture completion/error callbacks, TekuDefaultExceptionHandler, REST DefaultExceptionHandler, and ObservableValue / Subscribers when exceptions are suppressed. OOM exits use ERROR_EXIT_CODE (restart-friendly); existing fatal service/storage paths still use FATAL_EXIT_CODE. Gradle dependency rules allow infrastructure:subscribers to depend on exceptions.

^{Reviewed by Cursor Bugbot for commit 3946f6e. Bugbot is set up for automated code reviews on this repo. Configure here.}

[tbenr]

gpt found the issue i was mentioning: #7166

[tbenr]

some points of discussion

[tbenr]

in this context we need to be conservative: logging may throw too.
So:

try {
  statusLog.fatalError(subscriberDescription, exception);
} catch (final Throwable t) {
  System.err.println("Failed to log fatal error in " + subscriberDescription);
} finally {
  FatalErrorHandler.terminate(ERROR_EXIT_CODE);
}

[tbenr]

problem is that if the action itself throws it is possible we don't intercept that.
seems like all patterns following the functions\consumers should be wrapped in a try\catch

@Override
public SafeFuture<T> whenComplete(final BiConsumer<? super T, ? super Throwable> action) {
  return (SafeFuture<T>)
      super.whenComplete(
          (value, error) -> {
            final Throwable checked = checkForFatalError(error);
            try {
              action.accept(value, checked);
            } catch (final Throwable t) {
              checkForFatalError(t);
              throw t;
            }
          });
}

Then apply the same try/catch + checkForFatalError(t) pattern to exceptionally, handle, and handleAsync, and switch whenException / whenSuccess to go through whenComplete.
Also add checkForFatalError(t); before result.completeExceptionally(t) in handleComposed.

gpt is also suggesting to have something like:

public final class FatalErrorHandler {
  public static void runGuarded(final String context, final Runnable action) {
    try {
      action.run();
    } catch (final Throwable t) {
      shutdownIfFatalError(t, context);
      throw t;
    }
  }

  public static <T> T callGuarded(final String context, final Supplier<T> action) {
    try {
      return action.get();
    } catch (final Throwable t) {
      shutdownIfFatalError(t, context);
      throw t;
    }
  }
}

and then use that like:

public SafeFuture<T> whenSuccess(final Runnable action) {
  return whenComplete((value, error) -> {
    if (error == null) {
      FatalErrorHandler.runGuarded("SafeFuture.whenSuccess", action);
    }
  });
}

[zilm13]

makes sense, updating

[zilm13]

@tbenr all feedback addressed in d54affc

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 3946f6e. Configure here.}

[cursor]

OOM check overrides fatal exit code

Medium Severity

SafeFuture now calls shutdownIfFatalError on every exceptional completion, and shutdown always terminates with ERROR_EXIT_CODE. When an OutOfMemoryError is wrapped in FatalServiceFailureException or an unrecoverable DatabaseStorageException, that one-shot shutdown fires first, so TekuDefaultExceptionHandler cannot apply the intended FATAL_EXIT_CODE.

Additional Locations (2)

• infrastructure/async/src/main/java/tech/pegasys/teku/infrastructure/async/SafeFuture.java#L324-L330
• teku/src/main/java/tech/pegasys/teku/TekuDefaultExceptionHandler.java#L77-L91

 

^{Reviewed by Cursor Bugbot for commit 3946f6e. Configure here.}