diff --git a/bccsp/factory/factory.go b/bccsp/factory/factory.go index d8679d6..ca9576f 100644 --- a/bccsp/factory/factory.go +++ b/bccsp/factory/factory.go @@ -8,6 +8,7 @@ package factory import ( "sync" + "sync/atomic" "github.com/hyperledger/fabric-lib-go/bccsp" "github.com/hyperledger/fabric-lib-go/common/flogging" @@ -15,13 +16,13 @@ import ( ) 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) { diff --git a/bccsp/factory/factory_test.go b/bccsp/factory/factory_test.go index 5399d0b..56f3bb5 100644 --- a/bccsp/factory/factory_test.go +++ b/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") } diff --git a/bccsp/factory/nopkcs11.go b/bccsp/factory/nopkcs11.go index 6196a09..9310434 100644 --- a/bccsp/factory/nopkcs11.go +++ b/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. diff --git a/bccsp/factory/nopkcs11_test.go b/bccsp/factory/nopkcs11_test.go index cd2c059..9951922 100644 --- a/bccsp/factory/nopkcs11_test.go +++ b/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") diff --git a/bccsp/factory/pkcs11.go b/bccsp/factory/pkcs11.go index 44921cc..713e17a 100644 --- a/bccsp/factory/pkcs11.go +++ b/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. diff --git a/bccsp/factory/pkcs11_test.go b/bccsp/factory/pkcs11_test.go index a8463a9..d0c38ea 100644 --- a/bccsp/factory/pkcs11_test.go +++ b/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{}, })