Use atomic.Pointer to prevent race condition in bccsp.Factory

bccsp/factory/factory.go

@@ -8,20 +8,21 @@ package factory
 
 import (
 	"sync"
+	"sync/atomic"
 
 	"github.com/hyperledger/fabric-lib-go/bccsp"
 	"github.com/hyperledger/fabric-lib-go/common/flogging"
 	"github.com/pkg/errors"
 )
 
 var (
-	defaultBCCSP       bccsp.BCCSP // default BCCSP
-	factoriesInitOnce  sync.Once   // factories' Sync on Initialization
-	factoriesInitError error       // Factories' Initialization Error
+	defaultBCCSP       atomic.Pointer[bccsp.BCCSP] // default BCCSP
+	factoriesInitError atomic.Pointer[error]       // Factories' Initialization Error
+	factoriesInitOnce  sync.Once                   // Factories' Sync on Initialization
 
 	// when InitFactories has not been called yet (should only happen
 	// in test cases), use this BCCSP temporarily
-	bootBCCSP         bccsp.BCCSP
+	bootBCCSP         atomic.Pointer[bccsp.BCCSP]
 	bootBCCSPInitOnce sync.Once
 
 	logger = flogging.MustGetLogger("bccsp")
@@ -38,20 +39,49 @@ type BCCSPFactory interface {
 	Get(opts *FactoryOpts) (bccsp.BCCSP, error)
 }
 
+// InitFactories must be called before using factory interfaces
+// It is acceptable to call with config = nil, in which case
+// some defaults will get used.
+// Error is returned only if defaultBCCSP cannot be found.
+func InitFactories(config *FactoryOpts) error {
+	factoriesInitOnce.Do(func() {
+		res, err := initFactories(config)
+		if err != nil {
+			factoriesInitError.Store(&err)
+		}
+		if res != nil {
+			defaultBCCSP.Store(&res)
+		}
+	})
+	errPtr := factoriesInitError.Load()
+	if errPtr != nil {
+		return *errPtr
+	}
+	return nil
+}
+
 // GetDefault returns a non-ephemeral (long-term) BCCSP
 func GetDefault() bccsp.BCCSP {
-	if defaultBCCSP == nil {
+	res := defaultBCCSP.Load()
+	if res != nil {
+		return *res
+	}
+
+	bootBCCSPInitOnce.Do(func() {
 		logger.Debug("Before using BCCSP, please call InitFactories(). Falling back to bootBCCSP.")
-		bootBCCSPInitOnce.Do(func() {
-			var err error
-			bootBCCSP, err = (&SWFactory{}).Get(GetDefaultOpts())
-			if err != nil {
-				panic("BCCSP Internal error, failed initialization with GetDefaultOpts!")
-			}
-		})
-		return bootBCCSP
+		newBootBCCSP, err := (&SWFactory{}).Get(GetDefaultOpts())
+		if err != nil {
+			panic("BCCSP Internal error, failed initialization with GetDefaultOpts!")
+		}
+		bootBCCSP.Store(&newBootBCCSP)
+	})
+
+	res = bootBCCSP.Load()
+	if res != nil {
+		return *res
 	}
-	return defaultBCCSP
+	// This should never happen, but if it does, panic is better than returning nil.
+	panic("BCCSP Internal error, both defaultBCCSP and bootBCCSP are nil")
 }
 
 func initBCCSP(f BCCSPFactory, config *FactoryOpts) (bccsp.BCCSP, error) {

bccsp/factory/factory_test.go

@@ -10,8 +10,11 @@ import (
 	"fmt"
 	"os"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 
+	"github.com/hyperledger/fabric-lib-go/bccsp"
 	"github.com/hyperledger/fabric-lib-go/bccsp/pkcs11"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/require"
@@ -68,9 +71,9 @@ BCCSP:
 
 	for index, config := range cfgVariations {
 		fmt.Printf("Trying configuration [%d]\n", index)
-		factoriesInitError = initFactories(config)
-		if factoriesInitError != nil {
-			fmt.Fprintf(os.Stderr, "initFactories failed: %s", factoriesInitError)
+		initErr := InitFactories(config)
+		if initErr != nil {
+			fmt.Fprintf(os.Stderr, "initFactories failed: %s", initErr)
 			os.Exit(1)
 		}
 		if rc := m.Run(); rc != 0 {
@@ -81,6 +84,108 @@ BCCSP:
 }
 
 func TestGetDefault(t *testing.T) {
-	bccsp := GetDefault()
-	require.NotNil(t, bccsp, "Failed getting default BCCSP. Nil instance.")
+	bccspResult := GetDefault()
+	require.NotNil(t, bccspResult, "Failed getting default BCCSP. Nil instance.")
+}
+
+// TestGetDefaultAfterInitFactories verifies that GetDefault() returns
+// the properly initialized BCCSP after InitFactories() is called
+func TestGetDefaultAfterInitFactories(t *testing.T) {
+	// Reset state.
+	defaultBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	factoriesInitError = atomic.Pointer[error]{}
+	factoriesInitOnce = sync.Once{}
+	bootBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	bootBCCSPInitOnce = sync.Once{}
+
+	config := &FactoryOpts{
+		Default: "SW",
+		SW:      &SwOpts{Hash: "SHA2", Security: 256},
+	}
+
+	err := InitFactories(config)
+	require.NoError(t, err, "InitFactories() should succeed")
+
+	instance := GetDefault()
+	require.NotNil(t, instance, "GetDefault() should return initialized instance")
+
+	// Call again to ensure idempotency
+	instance2 := GetDefault()
+	require.Equal(t, instance, instance2, "Multiple calls should return same instance")
+}
+
+// TestBootBCCSPConcurrent verifies that the bootBCCSP fallback path
+// is thread-safe when GetDefault() is called before InitFactories().
+func TestBootBCCSPConcurrent(t *testing.T) {
+	// Reset state to simulate uninitialized factory.
+	defaultBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	factoriesInitError = atomic.Pointer[error]{}
+	factoriesInitOnce = sync.Once{}
+	bootBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	bootBCCSPInitOnce = sync.Once{}
+
+	const numGoroutines = 100
+	results := make([]bccsp.BCCSP, numGoroutines)
+
+	// Launch multiple goroutines that call GetDefault() before InitFactories()
+	// This should trigger the bootBCCSP fallback path.
+	var wg sync.WaitGroup
+	wg.Add(numGoroutines)
+	for i := range numGoroutines {
+		go func() {
+			defer wg.Done()
+			instance := GetDefault()
+			require.NotNil(t, instance, "GetDefault() should return bootBCCSP instance")
+			results[i] = instance
+		}()
+	}
+	wg.Wait()
+
+	// Verify all goroutines got the same bootBCCSP instance.
+	for _, r := range results {
+		require.Equal(t, results[0], r, "All goroutines should get the same bootBCCSP instance")
+	}
+}
+
+// TestConcurrentMixedAccess verifies thread safety when mixing
+// InitFactories() and GetDefault() calls concurrently.
+func TestConcurrentMixedAccess(t *testing.T) {
+	// Reset state.
+	defaultBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	factoriesInitError = atomic.Pointer[error]{}
+	factoriesInitOnce = sync.Once{}
+	bootBCCSP = atomic.Pointer[bccsp.BCCSP]{}
+	bootBCCSPInitOnce = sync.Once{}
+
+	const numGoroutines = 100
+	config := &FactoryOpts{
+		Default: "SW",
+		SW:      &SwOpts{Hash: "SHA2", Security: 256},
+	}
+
+	// Launch goroutines that mix InitFactories() and GetDefault() calls.
+	var wg sync.WaitGroup
+	wg.Add(numGoroutines * 2)
+	for range numGoroutines {
+		go func() {
+			defer wg.Done()
+			// Some goroutines call InitFactories
+			err := InitFactories(config)
+			require.NoError(t, err)
+			// All goroutines call GetDefault
+			instance := GetDefault()
+			require.NotNil(t, instance, "GetDefault() should never return nil")
+		}()
+		go func() {
+			defer wg.Done()
+			// All goroutines call GetDefault
+			instance := GetDefault()
+			require.NotNil(t, instance, "GetDefault() should never return nil")
+		}()
+	}
+	wg.Wait()
+
+	// Final verification
+	instance := GetDefault()
+	require.NotNil(t, instance, "Final GetDefault() should return valid instance")
 }

bccsp/factory/nopkcs11.go

@@ -25,19 +25,7 @@ type FactoryOpts struct {
 	SW      *SwOpts `json:"SW,omitempty" yaml:"SW,omitempty"`
 }
 
-// InitFactories must be called before using factory interfaces
-// It is acceptable to call with config = nil, in which case
-// some defaults will get used
-// Error is returned only if defaultBCCSP cannot be found
-func InitFactories(config *FactoryOpts) error {
-	factoriesInitOnce.Do(func() {
-		factoriesInitError = initFactories(config)
-	})
-
-	return factoriesInitError
-}
-
-func initFactories(config *FactoryOpts) error {
+func initFactories(config *FactoryOpts) (res bccsp.BCCSP, err error) {
 	// Take some precautions on default opts
 	if config == nil {
 		config = GetDefaultOpts()
@@ -54,18 +42,17 @@ func initFactories(config *FactoryOpts) error {
 	// Software-Based BCCSP
 	if config.Default == "SW" && config.SW != nil {
 		f := &SWFactory{}
-		var err error
-		defaultBCCSP, err = initBCCSP(f, config)
+		res, err = initBCCSP(f, config)
 		if err != nil {
-			return errors.Wrapf(err, "Failed initializing BCCSP")
+			return nil, errors.Wrapf(err, "Failed initializing BCCSP")
 		}
 	}
 
-	if defaultBCCSP == nil {
-		return errors.Errorf("Could not find default `%s` BCCSP", config.Default)
+	if res == nil {
+		return nil, errors.Errorf("Could not find default `%s` BCCSP", config.Default)
 	}
 
-	return nil
+	return res, nil
 }
 
 // GetBCCSPFromOpts returns a BCCSP created according to the options passed in input.

bccsp/factory/nopkcs11_test.go

@@ -16,13 +16,13 @@ import (
 )
 
 func TestInitFactories(t *testing.T) {
-	err := initFactories(&FactoryOpts{
+	_, err := initFactories(&FactoryOpts{
 		Default: "SW",
 		SW:      &SwOpts{},
 	})
 	require.EqualError(t, err, "Failed initializing BCCSP: Could not initialize BCCSP SW [Failed initializing configuration at [0,]: Hash Family not supported []]")
 
-	err = initFactories(&FactoryOpts{
+	_, err = initFactories(&FactoryOpts{
 		Default: "PKCS11",
 	})
 	require.EqualError(t, err, "Could not find default `PKCS11` BCCSP")

bccsp/factory/pkcs11.go

@@ -28,19 +28,7 @@ type FactoryOpts struct {
 	PKCS11  *pkcs11.PKCS11Opts `json:"PKCS11,omitempty" yaml:"PKCS11"`
 }
 
-// InitFactories must be called before using factory interfaces
-// It is acceptable to call with config = nil, in which case
-// some defaults will get used
-// Error is returned only if defaultBCCSP cannot be found
-func InitFactories(config *FactoryOpts) error {
-	factoriesInitOnce.Do(func() {
-		factoriesInitError = initFactories(config)
-	})
-
-	return factoriesInitError
-}
-
-func initFactories(config *FactoryOpts) error {
+func initFactories(config *FactoryOpts) (res bccsp.BCCSP, err error) {
 	// Take some precautions on default opts
 	if config == nil {
 		config = GetDefaultOpts()
@@ -57,28 +45,26 @@ func initFactories(config *FactoryOpts) error {
 	// Software-Based BCCSP
 	if config.Default == "SW" && config.SW != nil {
 		f := &SWFactory{}
-		var err error
-		defaultBCCSP, err = initBCCSP(f, config)
+		res, err = initBCCSP(f, config)
 		if err != nil {
-			return errors.Wrap(err, "Failed initializing SW.BCCSP")
+			return res, errors.Wrap(err, "Failed initializing SW.BCCSP")
 		}
 	}
 
 	// PKCS11-Based BCCSP
 	if config.Default == "PKCS11" && config.PKCS11 != nil {
 		f := &PKCS11Factory{}
-		var err error
-		defaultBCCSP, err = initBCCSP(f, config)
+		res, err = initBCCSP(f, config)
 		if err != nil {
-			return errors.Wrapf(err, "Failed initializing PKCS11.BCCSP")
+			return res, errors.Wrapf(err, "Failed initializing PKCS11.BCCSP")
 		}
 	}
 
-	if defaultBCCSP == nil {
-		return errors.Errorf("Could not find default `%s` BCCSP", config.Default)
+	if res == nil {
+		return res, errors.Errorf("Could not find default `%s` BCCSP", config.Default)
 	}
 
-	return nil
+	return res, nil
 }
 
 // GetBCCSPFromOpts returns a BCCSP created according to the options passed in input.

bccsp/factory/pkcs11_test.go

@@ -18,29 +18,29 @@ import (
 )
 
 func TestExportedInitFactories(t *testing.T) {
-	// Reset errors from previous negative test runs
-	factoriesInitError = initFactories(nil)
+	// Reset errors from previous negative test runs.
+	factoriesInitError.Store(nil)
 
 	err := InitFactories(nil)
 	require.NoError(t, err)
 }
 
 func TestInitFactories(t *testing.T) {
-	err := initFactories(nil)
+	_, err := initFactories(nil)
 	require.NoError(t, err)
 
-	err = initFactories(&FactoryOpts{})
+	_, err = initFactories(&FactoryOpts{})
 	require.NoError(t, err)
 }
 
 func TestInitFactoriesInvalidArgs(t *testing.T) {
-	err := initFactories(&FactoryOpts{
+	_, err := initFactories(&FactoryOpts{
 		Default: "SW",
 		SW:      &SwOpts{},
 	})
 	require.EqualError(t, err, "Failed initializing SW.BCCSP: Could not initialize BCCSP SW [Failed initializing configuration at [0,]: Hash Family not supported []]")
 
-	err = initFactories(&FactoryOpts{
+	_, err = initFactories(&FactoryOpts{
 		Default: "PKCS11",
 		PKCS11:  &pkcs11.PKCS11Opts{},
 	})