From 10ba29a0eb3cc7d72888691de5d0c80130f22fbc Mon Sep 17 00:00:00 2001 From: Changkun Ou Date: Wed, 22 Sep 2021 17:58:29 +0200 Subject: [PATCH] all: add backward support for Go 1.16 (#13) --- .github/workflows/clipboard.yml | 11 ++- clipboard_android.c | 1 + clipboard_android.go | 1 + clipboard_darwin.go | 1 + clipboard_darwin.m | 1 + clipboard_ios.go | 1 + clipboard_ios.m | 1 + clipboard_linux.c | 1 + clipboard_linux.go | 4 +- clipboard_nocgo.go | 1 + clipboard_windows.go | 1 + cmd/gclip-gui/main.go | 1 + example_test.go | 1 + go.mod | 7 +- internal/cgo/handle.go | 159 ++++++++++++++++++++++++++++++++ internal/cgo/handle_117.go | 16 ++++ internal/cgo/handle_test.go | 152 ++++++++++++++++++++++++++++++ vendor/modules.txt | 6 +- 18 files changed, 350 insertions(+), 16 deletions(-) create mode 100644 internal/cgo/handle.go create mode 100644 internal/cgo/handle_117.go create mode 100644 internal/cgo/handle_test.go diff --git a/.github/workflows/clipboard.yml b/.github/workflows/clipboard.yml index bc27c83..881ae61 100644 --- a/.github/workflows/clipboard.yml +++ b/.github/workflows/clipboard.yml @@ -21,6 +21,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] + go: [ '1.16.x', '1.17.x' ] steps: - name: Install and run dependencies (xvfb libx11-dev) if: ${{ runner.os == 'Linux' }} @@ -47,24 +48,24 @@ jobs: - uses: actions/setup-go@v2 with: stable: 'false' - go-version: '1.17.x' + go-version: ${{ matrix.go }} - - name: Build + - name: Build (${{ matrix.go }}) run: | go build -o gclip cmd/gclip/main.go go build -o gclip-gui cmd/gclip-gui/main.go - - name: Run Tests with CGO_ENABLED=1 + - name: Run Tests with CGO_ENABLED=1 (${{ matrix.go }}) if: ${{ runner.os == 'Linux' || runner.os == 'macOS'}} run: | CGO_ENABLED=1 go test -v -covermode=atomic . - - name: Run Tests with CGO_ENABLED=0 + - name: Run Tests with CGO_ENABLED=0 (${{ matrix.go }}) if: ${{ runner.os == 'Linux' || runner.os == 'macOS'}} run: | CGO_ENABLED=0 go test -v -covermode=atomic . - - name: Run Tests on Windows + - name: Run Tests on Windows (${{ matrix.go }}) if: ${{ runner.os == 'Windows'}} run: | go test -v -covermode=atomic . \ No newline at end of file diff --git a/clipboard_android.c b/clipboard_android.c index 9dc34f6..1f67c7a 100644 --- a/clipboard_android.c +++ b/clipboard_android.c @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build android +// +build android #include #include diff --git a/clipboard_android.go b/clipboard_android.go index d9d7c39..caf766b 100644 --- a/clipboard_android.go +++ b/clipboard_android.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build android +// +build android package clipboard diff --git a/clipboard_darwin.go b/clipboard_darwin.go index e104a06..ccd4a41 100644 --- a/clipboard_darwin.go +++ b/clipboard_darwin.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build darwin && !ios +// +build darwin,!ios package clipboard diff --git a/clipboard_darwin.m b/clipboard_darwin.m index e2d81a0..d076c4b 100644 --- a/clipboard_darwin.m +++ b/clipboard_darwin.m @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build darwin && !ios +// +build darwin,!ios // Interact with NSPasteboard using Objective-C // https://developer.apple.com/documentation/appkit/nspasteboard?language=objc diff --git a/clipboard_ios.go b/clipboard_ios.go index 112ffee..8881c9a 100644 --- a/clipboard_ios.go +++ b/clipboard_ios.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build ios +// +build ios package clipboard diff --git a/clipboard_ios.m b/clipboard_ios.m index 8a25596..2df9443 100644 --- a/clipboard_ios.m +++ b/clipboard_ios.m @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build ios +// +build ios #import #import diff --git a/clipboard_linux.c b/clipboard_linux.c index 8aec7b2..b4ce490 100644 --- a/clipboard_linux.c +++ b/clipboard_linux.c @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build linux && !android +// +build linux,!android #include #include diff --git a/clipboard_linux.go b/clipboard_linux.go index 67bb287..79df259 100644 --- a/clipboard_linux.go +++ b/clipboard_linux.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build linux && !android +// +build linux,!android package clipboard @@ -34,9 +35,10 @@ import ( "fmt" "os" "runtime" - "runtime/cgo" "time" "unsafe" + + "golang.design/x/clipboard/internal/cgo" ) const errmsg = `Failed to initialize the X11 display, and the clipboard package diff --git a/clipboard_nocgo.go b/clipboard_nocgo.go index e4a39ce..6ed796f 100644 --- a/clipboard_nocgo.go +++ b/clipboard_nocgo.go @@ -1,4 +1,5 @@ //go:build !windows && !cgo +// +build !windows,!cgo package clipboard diff --git a/clipboard_windows.go b/clipboard_windows.go index 5bb9e42..5996886 100644 --- a/clipboard_windows.go +++ b/clipboard_windows.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build windows +// +build windows package clipboard diff --git a/cmd/gclip-gui/main.go b/cmd/gclip-gui/main.go index 104d48f..b33d639 100644 --- a/cmd/gclip-gui/main.go +++ b/cmd/gclip-gui/main.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build android || ios || linux || darwin || windows +// +build android ios linux darwin windows // This is a very basic example for verification purpose that // demonstrates how the golang.design/x/clipboard can interact diff --git a/example_test.go b/example_test.go index 4da4270..e0ea63d 100644 --- a/example_test.go +++ b/example_test.go @@ -5,6 +5,7 @@ // Written by Changkun Ou //go:build cgo +// +build cgo package clipboard_test diff --git a/go.mod b/go.mod index ca5cf1e..bae8a05 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,8 @@ module golang.design/x/clipboard -go 1.17 +go 1.16 require ( golang.org/x/image v0.0.0-20210628002857-a66eb6448b8d golang.org/x/mobile v0.0.0-20210716004757-34ab1303b554 ) - -require ( - golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56 // indirect - golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect -) diff --git a/internal/cgo/handle.go b/internal/cgo/handle.go new file mode 100644 index 0000000..6340907 --- /dev/null +++ b/internal/cgo/handle.go @@ -0,0 +1,159 @@ +// Copyright 2021 The golang.design Initiative Authors. +// All rights reserved. Use of this source code is governed +// by a MIT license that can be found in the LICENSE file. +// +// Written by Changkun Ou + +//go:build !go1.17 +// +build !go1.17 + +// Package cgo is an implementation of golang.org/issue/37033. +// +// See golang.org/cl/294670 for code review discussion. +package cgo + +import ( + "reflect" + "sync" +) + +// Handle provides a safe representation to pass Go values between C and +// Go back and forth. The zero value of a handle is not a valid handle, +// and thus safe to use as a sentinel in C APIs. +// +// The underlying type of Handle may change, but the value is guaranteed +// to fit in an integer type that is large enough to hold the bit pattern +// of any pointer. For instance, on the Go side: +// +// package main +// +// /* +// extern void MyGoPrint(unsigned long long handle); +// void myprint(unsigned long long handle); +// */ +// import "C" +// import "runtime/cgo" +// +// //export MyGoPrint +// func MyGoPrint(handle C.ulonglong) { +// h := cgo.Handle(handle) +// val := h.Value().(int) +// println(val) +// h.Delete() +// } +// +// func main() { +// val := 42 +// C.myprint(C.ulonglong(cgo.NewHandle(val))) +// // Output: 42 +// } +// +// and on the C side: +// +// // A Go function +// extern void MyGoPrint(unsigned long long handle); +// +// // A C function +// void myprint(unsigned long long handle) { +// MyGoPrint(handle); +// } +type Handle uintptr + +// NewHandle returns a handle for a given value. If a given value is a +// pointer, slice, map, channel, or function that refers to the same +// object, the returned handle will also be the same. Besides, nil value +// must not be used. +// +// The handle is valid until the program calls Delete on it. The handle +// uses resources, and this package assumes that C code may hold on to +// the handle, so a program must explicitly call Delete when the handle +// is no longer needed. +// +// The intended use is to pass the returned handle to C code, which +// passes it back to Go, which calls Value. See an example in the +// comments of the Handle definition. +func NewHandle(v interface{}) Handle { + var k uintptr + + rv := reflect.ValueOf(v) + switch rv.Kind() { + case reflect.Ptr, reflect.UnsafePointer, reflect.Slice, + reflect.Map, reflect.Chan, reflect.Func: + if rv.IsNil() { + panic("cgo: cannot use Handle for nil value") + } + + k = rv.Pointer() + default: + // Wrap and turn a value parameter into a pointer. This enables + // us to always store the passing object as a pointer, and helps + // to identify which of whose are initially pointers or values + // when Value is called. + v = &wrap{v} + k = reflect.ValueOf(v).Pointer() + } + + // v was escaped to the heap because of reflection. As Go do not have + // a moving GC (and possibly lasts true for a long future), it is + // safe to use its pointer address as the key of the global map at + // this moment. The implementation must be reconsidered if moving GC + // is introduced internally in the runtime. + actual, loaded := m.LoadOrStore(k, v) + if !loaded { + return Handle(k) + } + + arv := reflect.ValueOf(actual) + switch arv.Kind() { + case reflect.Ptr, reflect.UnsafePointer, reflect.Slice, + reflect.Map, reflect.Chan, reflect.Func: + // The underlying object of the given Go value already have + // its existing handle. + if arv.Pointer() == k { + return Handle(k) + } + + // If the loaded pointer is inconsistent with the new pointer, + // it means the address has been used for different objects + // because of GC and its address is reused for a new Go object, + // meaning that the Handle does not call Delete explicitly when + // the old Go value is not needed. Consider this as a misuse of + // a handle, do panic. + panic("cgo: misuse of a Handle") + default: + panic("cgo: Handle implementation has an internal bug") + } +} + +// Delete invalidates a handle. This method must be called when C code no +// longer has a copy of the handle, and the program no longer needs the +// Go value that associated with the handle. +// +// The method panics if the handle is invalid already. +func (h Handle) Delete() { + _, ok := m.LoadAndDelete(uintptr(h)) + if !ok { + panic("cgo: misuse of an invalid Handle") + } +} + +// Value returns the associated Go value for a valid handle. +// +// The method panics if the handle is invalid already. +func (h Handle) Value() interface{} { + v, ok := m.Load(uintptr(h)) + if !ok { + panic("cgo: misuse of an invalid Handle") + } + if wv, ok := v.(*wrap); ok { + return wv.v + } + return v +} + +var m = &sync.Map{} // map[uintptr]interface{} + +// wrap wraps a Go value. +type wrap struct { + v interface{} +} diff --git a/internal/cgo/handle_117.go b/internal/cgo/handle_117.go new file mode 100644 index 0000000..a6dd9fb --- /dev/null +++ b/internal/cgo/handle_117.go @@ -0,0 +1,16 @@ +// Copyright 2021 The golang.design Initiative Authors. +// All rights reserved. Use of this source code is governed +// by a MIT license that can be found in the LICENSE file. +// +// Written by Changkun Ou + +//go:build go1.17 +// +build go1.17 + +package cgo + +import "runtime/cgo" + +type Handle = cgo.Handle + +var NewHandle = cgo.NewHandle diff --git a/internal/cgo/handle_test.go b/internal/cgo/handle_test.go new file mode 100644 index 0000000..3b4ea28 --- /dev/null +++ b/internal/cgo/handle_test.go @@ -0,0 +1,152 @@ +// Copyright 2021 The golang.design Initiative Authors. +// All rights reserved. Use of this source code is governed +// by a MIT license that can be found in the LICENSE file. +// +// Written by Changkun Ou + +//go:build !go1.17 +// +build !go1.17 + +package cgo + +import ( + "testing" +) + +func TestValueHandle(t *testing.T) { + v := 42 + + h1 := NewHandle(v) + h2 := NewHandle(v) + + if uintptr(h1) == uintptr(h2) { + t.Fatalf("duplicated Go values should have different handles") + } + + h1v := h1.Value().(int) + h2v := h2.Value().(int) + if h1v != h2v { + t.Fatalf("the Value of duplicated Go values are different: want %d, got %d", h1v, h2v) + } + if h1v != v { + t.Fatalf("the Value of a handle does not match origin: want %v, got %v", v, h1v) + } + + h1.Delete() + h2.Delete() + + siz := 0 + m.Range(func(k, v interface{}) bool { + siz++ + return true + }) + if siz != 0 { + t.Fatalf("handles are not deleted, want: %d, got %d", 0, siz) + } +} + +func TestPointerHandle(t *testing.T) { + v := 42 + + p1 := &v + p2 := &v + + h1 := NewHandle(p1) + h2 := NewHandle(p2) + + if uintptr(h1) != uintptr(h2) { + t.Fatalf("pointers to the same value should have same handle") + } + + h1v := h1.Value().(*int) + h2v := h2.Value().(*int) + if h1v != h2v { + t.Fatalf("the Value of a handle does not match origin: want %v, got %v", v, h1v) + } + + h1.Delete() + + siz := 0 + m.Range(func(k, v interface{}) bool { + siz++ + return true + }) + if siz != 0 { + t.Fatalf("handles are not deleted: want %d, got %d", 0, siz) + } + + defer func() { + if r := recover(); r != nil { + return + } + t.Fatalf("double Delete on a same handle did not trigger a panic") + }() + + h2.Delete() +} + +func TestNilHandle(t *testing.T) { + var v *int + + defer func() { + if r := recover(); r != nil { + return + } + t.Fatalf("nil should not be created as a handle successfully") + }() + + _ = NewHandle(v) +} + +func f1() {} +func f2() {} + +type foo struct{} + +func (f *foo) bar() {} +func (f *foo) wow() {} + +func TestFuncHandle(t *testing.T) { + h1 := NewHandle(f1) + h2 := NewHandle(f2) + h3 := NewHandle(f2) + + if h1 == h2 { + t.Fatalf("different functions should have different handles") + } + if h2 != h3 { + t.Fatalf("same functions should have same handles") + } + + f := foo{} + h4 := NewHandle(f.bar) + h5 := NewHandle(f.bar) + h6 := NewHandle(f.wow) + + if h4 != h5 { + t.Fatalf("same methods should have same handles") + } + + if h5 == h6 { + t.Fatalf("different methods should have different handles") + } +} +func BenchmarkHandle(b *testing.B) { + b.Run("non-concurrent", func(b *testing.B) { + for i := 0; i < b.N; i++ { + h := NewHandle(i) + _ = h.Value() + h.Delete() + } + }) + b.Run("concurrent", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + var v int + for pb.Next() { + h := NewHandle(v) + _ = h.Value() + h.Delete() + } + }) + }) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 06b7c7a..4934618 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,5 +1,4 @@ # golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56 -## explicit; go 1.12 golang.org/x/exp/shiny/driver/gldriver golang.org/x/exp/shiny/driver/internal/drawer golang.org/x/exp/shiny/driver/internal/errscreen @@ -9,13 +8,13 @@ golang.org/x/exp/shiny/driver/internal/win32 golang.org/x/exp/shiny/driver/internal/x11key golang.org/x/exp/shiny/screen # golang.org/x/image v0.0.0-20210628002857-a66eb6448b8d -## explicit; go 1.12 +## explicit golang.org/x/image/font golang.org/x/image/font/basicfont golang.org/x/image/math/f64 golang.org/x/image/math/fixed # golang.org/x/mobile v0.0.0-20210716004757-34ab1303b554 -## explicit; go 1.11 +## explicit golang.org/x/mobile/app golang.org/x/mobile/app/internal/callfn golang.org/x/mobile/event/key @@ -30,6 +29,5 @@ golang.org/x/mobile/geom golang.org/x/mobile/gl golang.org/x/mobile/internal/mobileinit # golang.org/x/sys v0.0.0-20210510120138-977fb7262007 -## explicit; go 1.17 golang.org/x/sys/internal/unsafeheader golang.org/x/sys/windows