Go binding: do not use pointer receiver for Close()

[gm42]

A mix of pointer/non-pointer receivers makes it impossible to use interfaces for the fdb.Database type; I propose to change the Close() method for consistency.

See https://go.dev/play/p/PQ_FDXajpnB for an example of the problem (needs to be run locally):

# command-line-arguments
./main.go:18:22: cannot use fdb.Database{} (value of type fdb.Database) as FoundationDB value in variable declaration: fdb.Database does not implement FoundationDB (method Close has pointer receiver)

Custom interfaces can still be used by using a pointer instead of directly the fdb.Database type:

var _ FoundationDB = &fdb.Database{}

But that's less efficient.

NOTE: no test was updated

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

• [x] The PR has a description, explaining both the problem and the solution.
• [x] The description mentions which forms of testing were done and the testing seems reasonable.
• [ ] Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

• [ ] This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
• [ ] There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

[gm42]

Cc @johscheuer

[foundationdb-ci]

Result of foundationdb-pr-clang-ide on Linux CentOS 7

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 0:21:36
• Result: :white_check_mark: SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 0:36:06
• Result: :white_check_mark: SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 0:43:05
• Result: :x: FAILED
• Error: Error while executing command: docker build --label "org.foundationdb.version=${FDB_VERSION}" --label "org.foundationdb.build_date=${BUILD_DATE}" --label "org.foundationdb.commit=${COMMIT_SHA}" --progress plain --build-arg FDB_VERSION="${FDB_VERSION}" --build-arg FDB_LIBRARY_VERSIONS="${FDB_VERSION}" --build-arg FDB_WEBSITE="${FDB_WEBSITE}" --tag foundationdb/ycsb:${FDB_VERSION}-${COMMIT_SHA}-debug --file Dockerfile.eks --target ycsb .. Reason: exit status 1
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)
• Cluster Test Logs zip file of the test logs (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos on macOS Ventura 13.x

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 0:47:18
• Result: :white_check_mark: SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-clang on Linux CentOS 7

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 1:03:39
• Result: :white_check_mark: SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr on Linux CentOS 7

• Commit ID: ae773d91570cf69deb46161d99617b50e74b3f11
• Duration 1:06:38
• Result: :white_check_mark: SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[gm42]

The error seems to be, while building a Docker image:

#23 ERROR: "/YCSB" not found: not found

Unrelated?

[gm42]

Please hold merging this PR, I have found an unrelated issue in main (cannot call Close() because destruction happens already via finalizer) and I am looking into it.

[gm42]

I have verified that calling fdb_database_destroy multiple times eventually crashes with a SIGSEGV, so what is written in the C API documentation is correct:

It must be called exactly once for each successful call to fdb_create_database().

However for unknown reasons it does not crash if it is called 1-2 extra times, while it certainly crashes if we do that in a loop:

--- a/bindings/go/src/fdb/fdb_test.go
+++ b/bindings/go/src/fdb/fdb_test.go
@@ -344,6 +344,11 @@ func TestDatabaseCloseRemovesResources(t *testing.T) {
        if db == newDB {
                t.Fatalf("Expected a different database object, got: %v and %v\n", db, newDB)
        }
+
+       for i := 1; i < 1000; i++ {
+               fmt.Printf("closed %d times so far, going to close again\n", i)
+               db.Close()
+       }
 }
 
 func ExampleOpenWithConnectionString() {

The above change will crash:

closed 1 times so far, going to close again
closed 2 times so far, going to close again
SIGSEGV: segmentation violation
PC=0x764c33e5583d m=0 sigcode=128 addr=0x0
signal arrived during cgo execution

goroutine 6 gp=0xc000007dc0 m=0 mp=0x675580 [syscall]:
runtime.cgocall(0x51b860, 0xc00006ddc8)
	/usr/local/go/src/runtime/cgocall.go:157 +0x4b fp=0xc00006dda0 sp=0xc00006dd68 pc=0x40844b
github.com/apple/foundationdb/bindings/go/src/fdb._Cfunc_fdb_database_destroy(0x1b2d650)
	_cgo_gotypes.go:197 +0x3f fp=0xc00006ddc8 sp=0xc00006dda0 pc=0x50fedf
github.com/apple/foundationdb/bindings/go/src/fdb.(*database).destroy.func1(0x58fae0?)
	/home/user/source/foundationdb/bindings/go/src/fdb/database.go:89 +0x3e fp=0xc00006de08 sp=0xc00006ddc8 pc=0x5119de
github.com/apple/foundationdb/bindings/go/src/fdb.(*database).destroy(0xc000014448)
	/home/user/source/foundationdb/bindings/go/src/fdb/database.go:89 +0x7c fp=0xc00006de80 sp=0xc00006de08 pc=0x51191c
github.com/apple/foundationdb/bindings/go/src/fdb.(*Database).Close(...)
	/home/user/source/foundationdb/bindings/go/src/fdb/database.go:73
github.com/apple/foundationdb/bindings/go/src/fdb_test.TestDatabaseCloseRemovesResources(0xc0001224e0)
	/home/user/source/foundationdb/bindings/go/src/fdb/fdb_test.go:350 +0x2a9 fp=0xc00006df70 sp=0xc00006de80 pc=0x519f29
testing.tRunner(0xc0001224e0, 0x566970)
	/usr/local/go/src/testing/testing.go:1689 +0xfb fp=0xc00006dfc0 sp=0xc00006df70 pc=0x4d0fbb
testing.(*T).Run.gowrap1()
	/usr/local/go/src/testing/testing.go:1742 +0x25 fp=0xc00006dfe0 sp=0xc00006dfc0 pc=0x4d1fe5
runtime.goexit({})
	/usr/local/go/src/runtime/asm_amd64.s:1695 +0x1 fp=0xc00006dfe8 sp=0xc00006dfe0 pc=0x473f01
created by testing.(*T).Run in goroutine 1
	/usr/local/go/src/testing/testing.go:1742 +0x390

goroutine 1 gp=0xc0000061c0 m=nil [chan receive]:
runtime.gopark(0xc000108820?, 0xc0001088a8?, 0x20?, 0x88?, 0x539c00?)
	/usr/local/go/src/runtime/proc.go:402 +0xce fp=0xc0001359c0 sp=0xc0001359a0 pc=0x43face
runtime.chanrecv(0xc00001e230, 0xc000135aa7, 0x1)
	/usr/local/go/src/runtime/chan.go:583 +0x3bf fp=0xc000135a38 sp=0xc0001359c0 pc=0x40aa3f
runtime.chanrecv1(0x674900?, 0x52c680?)
	/usr/local/go/src/runtime/chan.go:442 +0x12 fp=0xc000135a60 sp=0xc000135a38 pc=0x40a672
testing.(*T).Run(0xc000122340, {0x560e86?, 0x0?}, 0x566970)
	/usr/local/go/src/testing/testing.go:1750 +0x3ab fp=0xc000135b20 sp=0xc000135a60 pc=0x4d1e8b
testing.runTests.func1(0xc000122340)
	/usr/local/go/src/testing/testing.go:2161 +0x37 fp=0xc000135b60 sp=0xc000135b20 pc=0x4d3f77
testing.tRunner(0xc000122340, 0xc000135c70)
	/usr/local/go/src/testing/testing.go:1689 +0xfb fp=0xc000135bb0 sp=0xc000135b60 pc=0x4d0fbb
testing.runTests(0xc0000120c0, {0x65d220, 0x9, 0x9}, {0x1?, 0x4b97ce?, 0x674ae0?})
	/usr/local/go/src/testing/testing.go:2159 +0x445 fp=0xc000135ca0 sp=0xc000135bb0 pc=0x4d3e65
testing.(*M).Run(0xc00010a0a0)
	/usr/local/go/src/testing/testing.go:2027 +0x68b fp=0xc000135ed0 sp=0xc000135ca0 pc=0x4d286b
main.main()
	_testmain.go:79 +0x16c fp=0xc000135f50 sp=0xc000135ed0 pc=0x51b74c
runtime.main()
	/usr/local/go/src/runtime/proc.go:271 +0x29d fp=0xc000135fe0 sp=0xc000135f50 pc=0x43f67d
runtime.goexit({})
	/usr/local/go/src/runtime/asm_amd64.s:1695 +0x1 fp=0xc000135fe8 sp=0xc000135fe0 pc=0x473f01

The reason I am looking into this is because I experience, randomly, a crash when using the Close() method and I believe it is caused by this Close() method being always called during GC.

@johscheuer no blocker to merge this PR but I might want to submit another one to fix this issue e.g. GC's call to destroy() and user-initiated Close() should not cause SIGSEGVs. I believe it can be done using an atomic value, or by removing the finalizer and asking user to always call Close() explicitly.

I think the latter is the more idiomatic approach.

[gm42]

Created PR here: https://github.com/apple/foundationdb/pull/11394