Fix nil panic on lifecycle OnStart/OnStop (#917)

If fx lifecycle OnStart/OnStop is called with a nil context, we
panic.

Added tests that test this.

Also tagging another release with this fix.

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [1.18.1] = 2022-08-08
+### Fixed
+- Fix a nil panic when `nil` is passed to `OnStart` and `OnStop` lifecycle methods.
+
 ## [1.18.0] - 2022-08-05
 ### Added
 - Soft value groups that lets you specify value groups as best-effort dependencies.

internal/lifecycle/lifecycle.go

@@ -22,6 +22,7 @@ package lifecycle
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -72,6 +73,10 @@ func (l *Lifecycle) Append(hook Hook) {
 // Start runs all OnStart hooks, returning immediately if it encounters an
 // error.
 func (l *Lifecycle) Start(ctx context.Context) error {
+	if ctx == nil {
+		return errors.New("called OnStart with nil context")
+	}
+
 	l.mu.Lock()
 	l.startRecords = make(HookRecords, 0, len(l.hooks))
 	l.mu.Unlock()
@@ -129,6 +134,10 @@ func (l *Lifecycle) runStartHook(ctx context.Context, hook Hook) (runtime time.D
 // Stop runs any OnStop hooks whose OnStart counterpart succeeded. OnStop
 // hooks run in reverse order.
 func (l *Lifecycle) Stop(ctx context.Context) error {
+	if ctx == nil {
+		return errors.New("called OnStop with nil context")
+	}
+
 	l.mu.Lock()
 	l.stopRecords = make(HookRecords, 0, l.numStarted)
 	l.mu.Unlock()

internal/lifecycle/lifecycle_test.go

@@ -291,6 +291,32 @@ func TestLifecycleStop(t *testing.T) {
 		cancel()
 		require.Error(t, l.Stop(ctx))
 	})
+
+	t.Run("nil ctx", func(t *testing.T) {
+		t.Parallel()
+
+		l := New(testLogger(t), fxclock.System)
+		l.Append(Hook{
+			OnStart: func(context.Context) error {
+				assert.Fail(t, "this hook should not run")
+				return nil
+			},
+			OnStop: func(context.Context) error {
+				assert.Fail(t, "this hook should not run")
+				return nil
+			},
+		})
+		//lint:ignore SA1012 this test specifically tests for the lint failure
+		err := l.Start(nil)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "called OnStart with nil context")
+
+		//lint:ignore SA1012 this test specifically tests for the lint failure
+		err = l.Stop(nil)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "called OnStop with nil context")
+
+	})
 }
 
 func TestHookRecordsFormat(t *testing.T) {

version.go

@@ -21,4 +21,4 @@
 package fx
 
 // Version is exported for runtime compatibility checks.
-const Version = "1.18.0"
+const Version = "1.18.1"