Fix 2 problems in client_tcert_pool_mt.go, including one causing deadlock.

Fixed 2 obvious problems in client_tcert_pool_mt.go.

- Function (*tCertPoolEntry).GetNextTCert() should be thread-safe,
  so that it doesn't make sense to put tCertBlock to shared object field.
- Function (*tCertPoolMultithreadingImpl).Start() shouldn't call poolEntry.Start(),
  as it was already called inside tCertPool.getPoolEntry(attributes).

The latter problem was causing deadlock (due to insufficient channel size).
To reproduce, run go test -run=TestClientGetNextTCerts at crypto directory,
using a version of crypto_test.go in this commit
(reverting #1809).

This commit also includes a fix of TestClientGetAttributesFromTCertWithUnusedTCerts,
(using fresh client, not reusing the old one).

Change-Id: I1b4bd0dd111ebaf182a87dc5f3ecd3e4c0278a0f
Signed-off-by: Akihiko Tozawa <[email protected]>

Author: akihikot

core/crypto/client_tcert_pool_mt.go

@@ -30,15 +30,14 @@ type tCertPoolEntry struct {
 	tCertChannelFeedback chan struct{}
 	done                 chan struct{}
 	client               *clientImpl
-	tCertBlock           *TCertBlock
 }
 
 //NewTCertPoolEntry creates a new tcert pool entry
 func newTCertPoolEntry(client *clientImpl, attributes []string) *tCertPoolEntry {
 	tCertChannel := make(chan *TCertBlock, client.conf.getTCertBatchSize()*2)
 	tCertChannelFeedback := make(chan struct{}, client.conf.getTCertBatchSize()*2)
 	done := make(chan struct{}, 1)
-	return &tCertPoolEntry{attributes, tCertChannel, tCertChannelFeedback, done, client, nil}
+	return &tCertPoolEntry{attributes, tCertChannel, tCertChannelFeedback, done, client}
 }
 
 //Start starts the pool entry filler loop.
@@ -85,25 +84,24 @@ func (tCertPoolEntry *tCertPoolEntry) GetNextTCert(attributes ...string) (tCertB
 	for i := 0; i < 3; i++ {
 		tCertPoolEntry.client.Debugf("Getting next TCert... %d out of 3", i)
 		select {
-		case tCertPoolEntry.tCertBlock = <-tCertPoolEntry.tCertChannel:
+		case tCertBlock = <-tCertPoolEntry.tCertChannel:
 			break
 		case <-time.After(30 * time.Second):
 			tCertPoolEntry.client.Error("Failed getting a new TCert. Buffer is empty!")
 		}
-		if tCertPoolEntry.tCertBlock != nil {
+		if tCertBlock != nil {
 			// Send feedback to the filler
 			tCertPoolEntry.client.Debug("Send feedback")
 			tCertPoolEntry.tCertChannelFeedback <- struct{}{}
 			break
 		}
 	}
 
-	if tCertPoolEntry.tCertBlock == nil {
+	if tCertBlock == nil {
 		// TODO: change error here
 		return nil, errors.New("Failed getting a new TCert. Buffer is empty!")
 	}
 
-	tCertBlock = tCertPoolEntry.tCertBlock
 	tCertPoolEntry.client.Debugf("Cert [% x].", tCertBlock.tCert.GetCertificate().Raw)
 
 	// Store the TCert permanently
@@ -233,11 +231,8 @@ type tCertPoolMultithreadingImpl struct {
 func (tCertPool *tCertPoolMultithreadingImpl) Start() (err error) {
 	// Start the filler, initializes a poolEntry without attributes.
 	var attributes []string
-	poolEntry, err := tCertPool.getPoolEntry(attributes)
-	if err != nil {
-		return err
-	}
-	return poolEntry.Start()
+	_, err = tCertPool.getOrCreatePoolEntry(attributes)
+	return
 }
 
 func (tCertPool *tCertPoolMultithreadingImpl) lockEntries() {
@@ -273,7 +268,7 @@ func (tCertPool *tCertPoolMultithreadingImpl) getPoolEntryFromHash(attributeHash
 }
 
 //Returns a tCertPoolEntry for the attributes "attributes", if the tCertPoolEntry doesn't exists a new tCertPoolEntry will be create for that attributes.
-func (tCertPool *tCertPoolMultithreadingImpl) getPoolEntry(attributes []string) (*tCertPoolEntry, error) {
+func (tCertPool *tCertPoolMultithreadingImpl) getOrCreatePoolEntry(attributes []string) (*tCertPoolEntry, error) {
 	tCertPool.client.Debug("Getting pool entry %v \n", attributes)
 	attributeHash := calculateAttributesHash(attributes)
 	tCertPool.lockEntries()
@@ -307,7 +302,7 @@ func (tCertPool *tCertPoolMultithreadingImpl) GetNextTCerts(nCerts int, attribut
 }
 
 func (tCertPool *tCertPoolMultithreadingImpl) getNextTCert(attributes ...string) (tCertBlock *TCertBlock, err error) {
-	poolEntry, err := tCertPool.getPoolEntry(attributes)
+	poolEntry, err := tCertPool.getOrCreatePoolEntry(attributes)
 	if err != nil {
 		return nil, err
 	}

core/crypto/crypto_test.go

@@ -83,6 +83,14 @@ func TestMain(m *testing.M) {
 		os.Exit(ret)
 	}
 
+	//Second scenario with multithread
+	properties["security.multithreading.enabled"] = "true"
+	ret = runTestsOnScenario(m, properties, "Using multithread enabled")
+	if ret != 0 {
+		os.Exit(ret)
+	}
+	properties["security.multithreading.enabled"] = "false"
+
 	//Fourth scenario with security level = 384
 	properties["security.hashAlgorithm"] = "SHA3"
 	properties["security.level"] = "384"
@@ -432,6 +440,9 @@ func TestClientMultiExecuteTransaction(t *testing.T) {
 
 func TestClientGetNextTCerts(t *testing.T) {
 
+	initNodes()
+	defer closeNodes()
+
 	// Some positive flow tests here
 	var nCerts int = 1
 	for i := 1; i < 3; i++ {
@@ -509,8 +520,8 @@ func TestClientGetAttributesFromTCertWithUnusedTCerts(t *testing.T) {
 
 	_, _ = deployer.GetNextTCerts(1, attrs...)
 
-	after()  //Tear down the server.
-	before() //Start up again to use unsed TCerts
+	CloseAllClients() // Remove client and store unused TCerts.
+	initClients()     // Restart fresh client.
 
 	tcerts, err := deployer.GetNextTCerts(1, attrs...)