scrap the tenant finalizer, fixes apple#11856.

  several SetFinalizer time-bombs remain. We
  are working to get rid of them.

  fix broken OpenTenant().

  add db.TenantExists(), db.EnsureTenant(), db.DeleteTenantsIfExist().

  fix a bunch of bugs, races, and design problems

  adds back tenant.go; why is it missing on main branch?

bindings/go/src/fdb/database.go

@@ -29,6 +29,7 @@ import "C"
 import (
 	"errors"
 	"runtime"
+	"sync"
 )
 
 // Database is a handle to a FoundationDB database. Database is a lightweight
@@ -40,17 +41,29 @@ import (
 // usually created and committed automatically by the (Database).Transact
 // method.
 type Database struct {
+	// Do not put any member variables in here!
+	// Their updates will be lost since Database is value based.
+	// Any new fields must go inside the database struct. Being
+	// pointer based, updates to those fields will not be lost.
+	*database
+}
+
+type database struct {
+
 	// String reference to the cluster file.
 	clusterFile string
 	// This variable is to track if we have to remove the database from the cached
 	// database structs. We can't use clusterFile alone, since the default clusterFile
 	// would be an empty string.
 	isCached bool
-	*database
-}
 
-type database struct {
-	ptr *C.FDBDatabase
+	// Remember that sync.Mutex cannot be copied.
+	// Therefore, mut must live in a pointed-to
+	// struct such as tenant, rather than a passed
+	// by value struct such as Tenant.
+	mut  sync.Mutex
+	gone bool
+	ptr  *C.FDBDatabase
 }
 
 // DatabaseOptions is a handle with which to set options that affect a Database
@@ -61,15 +74,26 @@ type DatabaseOptions struct {
 }
 
 // Close will close the Database and clean up all resources.
-// It must be called exactly once for each created database.
-// You have to ensure that you're not reusing this database
-// after it has been closed.
-func (d Database) Close() {
-	// Remove database object from the cached databases
+// You have to ensure that you're not using the database
+// after it has been closed. It is safe to call Close()
+// multiple times and/or from multiple goroutines; the
+// underlying database will only be closed once.
+func (d *Database) Close() {
+
+	// make Close() goroutine safe, and idempotent.
+	d.mut.Lock()
+	defer d.mut.Unlock()
+	if d.gone {
+		return
+	}
+	d.gone = true
+
+	// Remove database object from the cached databases.
+	// reading d.isCached is obviously not thread/race-safe
+	// without the lock above.
 	if d.isCached {
 		openDatabases.Delete(d.clusterFile)
 	}
-
 	if d.ptr == nil {
 		return
 	}

bindings/go/src/fdb/errors.go

@@ -56,4 +56,8 @@ var (
 	errAPIVersionUnset        = Error{2200}
 	errAPIVersionAlreadySet   = Error{2201}
 	errAPIVersionNotSupported = Error{2203}
+
+	errTenantNotFound    = Error{2131}
+	errTenantExists      = Error{2132}
+	errTenantNameInvalid = Error{2134}
 )

bindings/go/src/fdb/fdb.go

@@ -330,6 +330,7 @@ func MustOpenDefault() Database {
 // the multi-version client API must be used.
 // Caller must call Close() to release resources.
 func OpenDatabase(clusterFile string) (Database, error) {
+
 	var db Database
 	var okDb bool
 	anyy, exist := openDatabases.Load(clusterFile)
@@ -401,9 +402,9 @@ func createDatabase(clusterFile string) (Database, error) {
 		return Database{}, createErr
 	}
 
-	db := &database{outdb}
+	db := &database{ptr: outdb, clusterFile: clusterFile, isCached: true}
 
-	return Database{clusterFile: clusterFile, isCached: true, database: db}, nil
+	return Database{database: db}, nil
 }
 
 // OpenWithConnectionString returns a database handle to the FoundationDB cluster identified
@@ -437,9 +438,8 @@ func OpenWithConnectionString(connectionString string) (Database, error) {
 		return Database{}, createErr
 	}
 
-	db := &database{outdb}
-
-	return Database{"", false, db}, nil
+	db := &database{ptr: outdb}
+	return Database{database: db}, nil
 }
 
 // Deprecated: Use OpenDatabase instead.

bindings/go/src/fdb/fdb_test.go

@@ -23,6 +23,7 @@
 package fdb_test
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"testing"
@@ -389,3 +390,231 @@ func ExampleOpenWithConnectionString() {
 
 	// Output:
 }
+
+// Copied from errors.go so that these types aren't public
+var (
+	errTenantNotFound    = fdb.Error{2131}
+	errTenantExists      = fdb.Error{2132}
+	errTenantNameInvalid = fdb.Error{2134}
+)
+
+func TestCreateTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName := fdb.Key("test-create-tenant")
+
+	exists, err := db.TenantExists(testTenantName)
+	if err != nil {
+		t.Fatalf("Unable to check if tenant exists: %v\n", err)
+	}
+	//fmt.Printf("Tenant exists for '%v': %v\n", string(testTenantName), exists)
+
+	if !exists {
+		err := db.CreateTenant(testTenantName)
+		if err != nil {
+			t.Fatalf("Unable to create tenant: %v\n", err)
+		}
+	}
+
+	_, err = db.OpenTenant(testTenantName)
+	if err != nil {
+		t.Fatalf("Unable to open tenant: %v\n", err)
+	}
+}
+
+func TestDeleteTenantsIfExist(t *testing.T) {
+
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	// 1. get a clean baseline first, so no test cross talk.
+
+	testTenantName1 := fdb.Key("1-bulk-delete-test-list-1-tenant-1")
+	testTenantName2 := fdb.Key("1-bulk-delete-test-list-1-tenant-2")
+
+	names := []fdb.KeyConvertible{testTenantName1, testTenantName2}
+	deleteBoth := func(t *testing.T) {
+		err := db.DeleteTenantsIfExist(names)
+		if err != nil {
+			t.Fatalf("Unable to delete existing tenants if they exist: %v\n", err)
+		}
+	}
+	deleteBoth(t)
+
+	// 2. verify these tenants do not exists, using ListTenants().
+
+	mustNotExist := func(t *testing.T) {
+		ls, err := db.ListTenants()
+		if err != nil {
+			t.Fatalf("Unable to list tenants: %v\n", err)
+		}
+
+		if inSlice(ls, testTenantName1) {
+			t.Fatalf("tenant 1 still in slice %#v", ls)
+		}
+
+		if inSlice(ls, testTenantName2) {
+			t.Fatalf("tenant 2 still in slice, %#v", ls)
+		}
+	}
+	mustNotExist(t)
+
+	// 3. create one of them but not the other
+
+	err := db.CreateTenant(testTenantName1)
+	if err != nil {
+		t.Fatalf("Unable to create testTenantName1: %v\n", err)
+	}
+
+	// 4. use TenantExists to test it as well.
+	exists1, err := db.TenantExists(testTenantName1)
+	if err != nil {
+		t.Fatalf("Unable to check if tenant1 exists: %v\n", err)
+	}
+	if !exists1 {
+		t.Fatalf("testTenantName1 '%v' should exist now, but does not.", string(testTenantName1))
+	}
+
+	exists2, err := db.TenantExists(testTenantName2)
+	if err != nil {
+		t.Fatalf("Unable to check if tenant2 exists: %v\n", err)
+	}
+	if exists2 {
+		t.Fatalf("testTenantName2 '%v' should not exist now, but does.", string(testTenantName2))
+	}
+
+	// 5. delete them both, this verifies there was no error
+	//    that the second one did not exist
+	deleteBoth(t)
+
+	// 6. verify both are gone.
+	mustNotExist(t)
+}
+
+func TestCreateExistTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName := fdb.Key("test-exist-tenant")
+
+	// first, cleanup any prior tenant of this name
+
+	exists, err := db.TenantExists(testTenantName)
+	if err != nil {
+		t.Fatalf("Unable to check if tenant exists: %v\n", err)
+	}
+	//fmt.Printf("Tenant exists for '%v': %v\n", string(testTenantName), exists)
+
+	if exists {
+		err = db.DeleteTenant(testTenantName)
+		if err != nil {
+			t.Fatalf("Unable to delete existing tenant '%v': %v\n", string(testTenantName), err)
+		}
+	}
+
+	err = db.CreateTenant(testTenantName)
+	if err != nil {
+		t.Fatalf("Unable to create tenant: %v\n", err)
+	}
+
+	// This should fail
+	err = db.CreateTenant(testTenantName)
+	assertErrorCodeEqual(t, err, errTenantExists)
+}
+
+func TestOpenNotExistTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName := fdb.Key("test-not-exist-tenant")
+
+	// this should fail
+	_, err := db.OpenTenant(testTenantName)
+	assertErrorCodeEqual(t, err, errTenantNotFound)
+}
+
+func TestDeleteNotExistTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName := fdb.Key("test-not-exist-tenant")
+
+	// this should fail
+	err := db.DeleteTenant(testTenantName)
+	assertErrorCodeEqual(t, err, errTenantNotFound)
+}
+
+func inSlice(sl []fdb.Key, t fdb.Key) bool {
+	for _, s := range sl {
+		if bytes.Equal(s, t) {
+			return true
+		}
+	}
+	return false
+}
+
+func assertErrorCodeEqual(t *testing.T, actual error, expected fdb.Error) {
+	if actual == nil {
+		t.Fatalf("Error is nil when it should be: %v\n", expected.Code)
+	}
+
+	castErr, ok := actual.(fdb.Error)
+	if !ok {
+		t.Fatalf("Error is wrong type %v, expected %v\n", actual, expected)
+	}
+
+	if castErr.Code != expected.Code {
+		t.Fatalf("Error is wrong code, expected %v, actual %v\n", expected.Code, castErr.Code)
+	}
+}
+
+func TestListTenant_EnsureTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName1 := fdb.Key("1-test-list-1-tenant-1")
+	testTenantName2 := fdb.Key("2-test-list-2-tenant-2")
+	names := []fdb.KeyConvertible{testTenantName1, testTenantName2}
+
+	err := db.DeleteTenantsIfExist(names)
+	if err != nil {
+		t.Fatalf("Unable to delete existing tenants if they exist: %v\n", err)
+	}
+
+	ten1, err := db.EnsureTenant(testTenantName1)
+	if err != nil {
+		t.Fatalf("Unable to create tenant 1: %v\n", err)
+	}
+	defer ten1.Close()
+
+	ten2, err := db.EnsureTenant(testTenantName2)
+	if err != nil {
+		t.Fatalf("Unable to create tenant 2: %v\n", err)
+	}
+	defer ten2.Close()
+
+	ls, err := db.ListTenants()
+	if err != nil {
+		t.Fatalf("Unable to list tenants: %v\n", err)
+	}
+
+	if !inSlice(ls, testTenantName1) {
+		t.Fatalf("tenant 1 not in slice %#v", ls)
+	}
+
+	if !inSlice(ls, testTenantName2) {
+		t.Fatalf("tenant 2 not in slice, %#v", ls)
+	}
+}
+
+func TestInvalidPrefixTenant(t *testing.T) {
+	fdb.MustAPIVersion(API_VERSION)
+	db := fdb.MustOpenDefault()
+
+	testTenantName := fdb.Key("\xFFtest-invalid-prefix-tenant")
+
+	// this should fail
+	err := db.CreateTenant(testTenantName)
+	assertErrorCodeEqual(t, err, errTenantNameInvalid)
+}