Skip to content

Commit

Permalink
[FAB-3555] Peer panic on SIGSEV connecting to CouchDB
Browse files Browse the repository at this point in the history
A peer panic was encountered connecting to CouchDB on
peer startup.   This occurred in the code after multiple
retries.

The panic was caused by a null pointer when attempting
to read the status code from an http return object.  The
http return object was already checked for an error.

The situation causing the null pointer can only be caused
if the golang http return has a nil for both the response
and the error.   According to golang spec this is not
possible.

An additional check is being made to ensure a good error
message is returned from the CouchDB connection request.

Change-Id: I13740a32231d49faf3a6acf7b37f7bc6e4bba3fa
Signed-off-by: Chris Elder <chris.elder@us.ibm.com>
  • Loading branch information
Chris Elder committed Jun 13, 2017
1 parent f5dbbaf commit dc8d45f
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 0 deletions.
25 changes: 25 additions & 0 deletions core/ledger/util/couchdb/couchdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,10 @@ func (couchInstance *CouchInstance) handleRequest(method, connectURL string, dat
//set initial wait duration for retries
waitDuration := retryWaitTime * time.Millisecond

if maxRetries < 1 {
return nil, nil, fmt.Errorf("Number of retries must be greater than zero.")
}

//attempt the http request for the max number of retries
for attempts := 0; attempts < maxRetries; attempts++ {

Expand Down Expand Up @@ -1285,6 +1289,11 @@ func (couchInstance *CouchInstance) handleRequest(method, connectURL string, dat
//Execute http request
resp, errResp = couchInstance.client.Do(req)

//check to see if the return from CouchDB is valid
if invalidCouchDBReturn(resp, errResp) {
continue
}

//if there is no golang http error and no CouchDB 500 error, then drop out of the retry
if errResp == nil && resp != nil && resp.StatusCode < 500 {
break
Expand Down Expand Up @@ -1329,6 +1338,14 @@ func (couchInstance *CouchInstance) handleRequest(method, connectURL string, dat
return nil, nil, errResp
}

//This situation should not occur according to the golang spec.
//if this error returned (errResp) from an http call, then the resp should be not nil,
//this is a structure and StatusCode is an int
//This is meant to provide a more graceful error if this should occur
if invalidCouchDBReturn(resp, errResp) {
return nil, nil, fmt.Errorf("Unable to connect to CouchDB, check the hostname and port.")
}

//set the return code for the couchDB request
couchDBReturn.StatusCode = resp.StatusCode

Expand Down Expand Up @@ -1364,6 +1381,14 @@ func (couchInstance *CouchInstance) handleRequest(method, connectURL string, dat
return resp, couchDBReturn, nil
}

//invalidCouchDBResponse checks to make sure either a valid response or error is returned
func invalidCouchDBReturn(resp *http.Response, errResp error) bool {
if resp == nil && errResp == nil {
return true
}
return false
}

//IsJSON tests a string to determine if a valid JSON
func IsJSON(s string) bool {
var js map[string]interface{}
Expand Down
22 changes: 22 additions & 0 deletions core/ledger/util/couchdb/couchdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,28 @@ func TestDBTimeoutConflictRetry(t *testing.T) {
}
}

func TestDBBadNumberOfRetries(t *testing.T) {

if ledgerconfig.IsCouchDBEnabled() {

database := "testdbbadretries"
err := cleanup(database)
testutil.AssertNoError(t, err, fmt.Sprintf("Error when trying to cleanup Error: %s", err))
defer cleanup(database)

// if there was an error upon cleanup, return here
if err != nil {
return
}

//create a new instance and database object
_, err = CreateCouchInstance(couchDBDef.URL, couchDBDef.Username, couchDBDef.Password,
0, 3, couchDBDef.RequestTimeout)
testutil.AssertError(t, err, fmt.Sprintf("Error should have been thrown while attempting to create a database"))

}
}

func TestDBBadJSON(t *testing.T) {

if ledgerconfig.IsCouchDBEnabled() {
Expand Down

0 comments on commit dc8d45f

Please sign in to comment.