Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client: hide pointer indirection. #260

Merged
merged 2 commits into from
Jul 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions answer.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type Promise struct {
}

type clientAndPromise struct {
client *Client
client Client
promise *ClientPromise
}

Expand Down Expand Up @@ -295,7 +295,7 @@ func (ans *Answer) Struct() (Struct, error) {
// call has not completed, then calls will be queued until the original
// call's completion. The client reference is borrowed: the caller
// should not call Close.
func (ans *Answer) Client() *Client {
func (ans *Answer) Client() Client {
return ans.f.Client()
}

Expand Down Expand Up @@ -423,7 +423,7 @@ func (f *Future) Struct() (Struct, error) {
// call has not completed, then calls will be queued until the original
// call's completion. The client reference is borrowed: the caller
// should not call Close.
func (f *Future) Client() *Client {
func (f *Future) Client() Client {
p := f.promise
p.mu.Lock()
switch {
Expand Down Expand Up @@ -585,7 +585,7 @@ func (r resolution) strct(transform []PipelineOp) (Struct, error) {
}

// client obtains a Client by applying a transform.
func (r resolution) client(transform []PipelineOp) *Client {
func (r resolution) client(transform []PipelineOp) Client {
p, err := r.ptr(transform)
if err != nil {
return ErrorClient(err)
Expand Down
100 changes: 54 additions & 46 deletions capability.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ func (i Interface) value(paddr address) rawPointer {

// Client returns the client stored in the message's capability table
// or nil if the pointer is invalid.
func (i Interface) Client() *Client {
func (i Interface) Client() Client {
msg := i.Message()
if msg == nil {
return nil
return Client{}
}
tab := msg.CapTable
if int64(i.cap) >= int64(len(tab)) {
return nil
return Client{}
}
return tab[i.cap]
}
Expand All @@ -96,6 +96,10 @@ func (id CapabilityID) GoString() string {
// The zero value is a null capability reference.
// It is safe to use from multiple goroutines.
type Client struct {
*client
}

type client struct {
creatorFunc int
creatorFile string
creatorLine int
Expand Down Expand Up @@ -134,9 +138,9 @@ type clientHook struct {
//
// Typically the RPC system will create a client for the application.
// Most applications will not need to use this directly.
func NewClient(hook ClientHook) *Client {
func NewClient(hook ClientHook) Client {
if hook == nil {
return nil
return Client{}
}
h := &clientHook{
ClientHook: hook,
Expand All @@ -146,11 +150,11 @@ func NewClient(hook ClientHook) *Client {
metadata: *NewMetadata(),
}
h.resolvedHook = h
c := &Client{h: h}
c := Client{client: &client{h: h}}
if clientLeakFunc != nil {
c.creatorFunc = 1
_, c.creatorFile, c.creatorLine, _ = runtime.Caller(1)
runtime.SetFinalizer(c, finalizeClient)
c.setFinalizer()
}
return c
}
Expand All @@ -162,7 +166,7 @@ func NewClient(hook ClientHook) *Client {
//
// Typically the RPC system will create a client for the application.
// Most applications will not need to use this directly.
func NewPromisedClient(hook ClientHook) (*Client, *ClientPromise) {
func NewPromisedClient(hook ClientHook) (Client, *ClientPromise) {
if hook == nil {
panic("NewPromisedClient(nil)")
}
Expand All @@ -173,20 +177,20 @@ func NewPromisedClient(hook ClientHook) (*Client, *ClientPromise) {
resolved: make(chan struct{}),
metadata: *NewMetadata(),
}
c := &Client{h: h}
c := Client{client: &client{h: h}}
if clientLeakFunc != nil {
c.creatorFunc = 2
_, c.creatorFile, c.creatorLine, _ = runtime.Caller(1)
runtime.SetFinalizer(c, finalizeClient)
c.setFinalizer()
}
return c, &ClientPromise{h: h}
}

// startCall holds onto a hook to prevent it from shutting down until
// finish is called. It resolves the client's hook as much as possible
// first. The caller must not be holding onto c.mu.
func (c *Client) startCall() (hook ClientHook, resolved, released bool, finish func()) {
if c == nil {
func (c Client) startCall() (hook ClientHook, resolved, released bool, finish func()) {
if c.client == nil {
return nil, true, false, func() {}
}
defer c.mu.Unlock()
Expand All @@ -212,8 +216,8 @@ func (c *Client) startCall() (hook ClientHook, resolved, released bool, finish f
}
}

func (c *Client) peek() (hook *clientHook, released bool, resolved bool) {
if c == nil {
func (c Client) peek() (hook *clientHook, released bool, resolved bool) {
if c.client == nil {
return nil, false, true
}
defer c.mu.Unlock()
Expand Down Expand Up @@ -254,7 +258,7 @@ func resolveHook(h *clientHook) *clientHook {

// Get the current flowcontrol.FlowLimiter used to manage flow control
// for this client.
func (c *Client) GetFlowLimiter() flowcontrol.FlowLimiter {
func (c Client) GetFlowLimiter() flowcontrol.FlowLimiter {
c.mu.Lock()
defer c.mu.Unlock()
ret := c.limiter
Expand All @@ -268,7 +272,7 @@ func (c *Client) GetFlowLimiter() flowcontrol.FlowLimiter {
// this client. This affects all future calls, but not calls already
// waiting to send. Passing nil sets the value to flowcontrol.NopLimiter,
// which is also the default.
func (c *Client) SetFlowLimiter(lim flowcontrol.FlowLimiter) {
func (c Client) SetFlowLimiter(lim flowcontrol.FlowLimiter) {
c.mu.Lock()
defer c.mu.Unlock()
c.limiter = lim
Expand All @@ -281,7 +285,7 @@ func (c *Client) SetFlowLimiter(lim flowcontrol.FlowLimiter) {
//
// This method respects the flow control policy configured with SetFlowLimiter;
// it may block if the sender is sending too fast.
func (c *Client) SendCall(ctx context.Context, s Send) (*Answer, ReleaseFunc) {
func (c Client) SendCall(ctx context.Context, s Send) (*Answer, ReleaseFunc) {
h, _, released, finish := c.startCall()
defer finish()
if released {
Expand Down Expand Up @@ -359,7 +363,7 @@ func (c *Client) SendCall(ctx context.Context, s Send) (*Answer, ReleaseFunc) {
//
// Note that unlike SendCall, this method does *not* respect the flow
// control policy configured with SetFlowLimiter.
func (c *Client) RecvCall(ctx context.Context, r Recv) PipelineCaller {
func (c Client) RecvCall(ctx context.Context, r Recv) PipelineCaller {
h, _, released, finish := c.startCall()
defer finish()
if released {
Expand All @@ -376,7 +380,7 @@ func (c *Client) RecvCall(ctx context.Context, r Recv) PipelineCaller {
// IsValid reports whether c is a valid reference to a capability.
// A reference is invalid if it is nil, has resolved to null, or has
// been released.
func (c *Client) IsValid() bool {
func (c Client) IsValid() bool {
h, released, _ := c.peek()
return !released && h != nil
}
Expand All @@ -385,7 +389,7 @@ func (c *Client) IsValid() bool {
// same call to NewClient. This can return false negatives if c or c2
// are not fully resolved: use Resolve if this is an issue. If either
// c or c2 are released, then IsSame panics.
func (c *Client) IsSame(c2 *Client) bool {
func (c Client) IsSame(c2 Client) bool {
h1, released, _ := c.peek()
if released {
panic("IsSame on released client")
Expand All @@ -398,7 +402,7 @@ func (c *Client) IsSame(c2 *Client) bool {
}

// Resolve blocks until the capability is fully resolved or the Context is Done.
func (c *Client) Resolve(ctx context.Context) error {
func (c Client) Resolve(ctx context.Context) error {
for {
h, released, resolved := c.peek()
if released {
Expand All @@ -419,37 +423,37 @@ func (c *Client) Resolve(ctx context.Context) error {

// AddRef creates a new Client that refers to the same capability as c.
// If c is nil or has resolved to null, then AddRef returns nil.
func (c *Client) AddRef() *Client {
if c == nil {
return nil
func (c Client) AddRef() Client {
if c.client == nil {
return Client{}
}
defer c.mu.Unlock()
c.mu.Lock()
if c.released {
panic("AddRef on released client")
}
if c.h == nil {
return nil
return Client{}
}
c.h.mu.Lock()
c.h = resolveHook(c.h)
if c.h == nil {
return nil
return Client{}
}
c.h.refs++
c.h.mu.Unlock()
d := &Client{h: c.h}
d := Client{client: &client{h: c.h}}
if clientLeakFunc != nil {
d.creatorFunc = 3
_, d.creatorFile, d.creatorLine, _ = runtime.Caller(1)
runtime.SetFinalizer(d, finalizeClient)
d.setFinalizer()
}
return d
}

// WeakRef creates a new WeakClient that refers to the same capability
// as c. If c is nil or has resolved to null, then WeakRef returns nil.
func (c *Client) WeakRef() *WeakClient {
func (c Client) WeakRef() *WeakClient {
h, released, _ := c.peek()
if released {
panic("WeakRef on released client")
Expand All @@ -462,7 +466,7 @@ func (c *Client) WeakRef() *WeakClient {

// State reads the current state of the client. It returns the zero
// ClientState if c is nil, has resolved to null, or has been released.
func (c *Client) State() ClientState {
func (c Client) State() ClientState {
h, resolved, _, finish := c.startCall()
defer finish()
if h == nil {
Expand Down Expand Up @@ -499,8 +503,8 @@ type ClientState struct {
// purposes. Its format should not be depended on: in particular, it
// should not be used to compare clients. Use IsSame to compare clients
// for equality.
func (c *Client) String() string {
if c == nil {
func (c Client) String() string {
if c.client == nil {
return "<nil>"
}
c.mu.Lock()
Expand Down Expand Up @@ -535,8 +539,8 @@ func (c *Client) String() string {
//
// Release will panic if c has already been released, but not if c is
// nil or resolved to null.
func (c *Client) Release() {
if c == nil {
func (c Client) Release() {
if c.client == nil {
return
}
c.mu.Lock()
Expand Down Expand Up @@ -592,7 +596,11 @@ func SetClientLeakFunc(f func(msg string)) {
clientLeakFunc = f
}

func finalizeClient(c *Client) {
func (c Client) setFinalizer() {
runtime.SetFinalizer(c.client, finalizeClient)
}

func finalizeClient(c *client) {
// Since there are no other references to c, then we don't have to
// acquire the mutex to read.
if c.released {
Expand Down Expand Up @@ -636,10 +644,10 @@ func (cp *ClientPromise) Reject(err error) {
// NewPromisedClient will be shut down after Fulfill returns, but the
// hook may have been shut down earlier if the client ran out of
// references.
func (cp *ClientPromise) Fulfill(c *Client) {
func (cp *ClientPromise) Fulfill(c Client) {
// Obtain next client hook.
var rh *clientHook
if c != nil {
if (c != Client{}) {
c.mu.Lock()
if c.released {
c.mu.Unlock()
Expand Down Expand Up @@ -687,29 +695,29 @@ type WeakClient struct {

// AddRef creates a new Client that refers to the same capability as c
// as long as the capability hasn't already been shut down.
func (wc *WeakClient) AddRef() (c *Client, ok bool) {
func (wc *WeakClient) AddRef() (c Client, ok bool) {
if wc == nil {
return nil, true
return Client{}, true
}
if wc.h == nil {
return nil, true
return Client{}, true
}
wc.h.mu.Lock()
wc.h = resolveHook(wc.h)
if wc.h == nil {
return nil, true
return Client{}, true
}
if wc.h.refs == 0 {
wc.h.mu.Unlock()
return nil, false
return Client{}, false
}
wc.h.refs++
wc.h.mu.Unlock()
c = &Client{h: wc.h}
c = Client{client: &client{h: wc.h}}
if clientLeakFunc != nil {
c.creatorFunc = 3
_, c.creatorFile, c.creatorLine, _ = runtime.Caller(1)
runtime.SetFinalizer(c, finalizeClient)
c.setFinalizer()
}
return c, true
}
Expand Down Expand Up @@ -874,7 +882,7 @@ type errorClient struct {
//
// The returned client's State() method returns a State with its
// Brand.Value set to e.
func ErrorClient(e error) *Client {
func ErrorClient(e error) Client {
if e == nil {
panic("ErrorClient(nil)")
}
Expand All @@ -888,7 +896,7 @@ func ErrorClient(e error) *Client {
metadata: *NewMetadata(),
}
h.resolvedHook = h
return &Client{h: h}
return Client{client: &client{h: h}}
}

func (ec errorClient) Send(_ context.Context, s Send) (*Answer, ReleaseFunc) {
Expand Down
8 changes: 4 additions & 4 deletions capability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,19 @@ func TestReleasedClient(t *testing.T) {
func TestNullClient(t *testing.T) {
lthibault marked this conversation as resolved.
Show resolved Hide resolved
ctx := context.Background()
c, p := NewPromisedClient(new(dummyHook))
p.Fulfill(nil)
p.Fulfill(Client{})
tests := []struct {
name string
c *Client
c Client
}{
{"nil", nil},
{"nil", Client{}},
{"promised nil", c},
}

for _, test := range tests {
c := test.c
t.Run(test.name, func(t *testing.T) {
if NewClient(nil) != nil {
if (NewClient(nil) != Client{}) {
t.Error("NewClient(nil) != nil")
}
if !c.IsSame(c) {
Expand Down
2 changes: 1 addition & 1 deletion capnpc-go/templates/interfaceClient
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{with .Annotations.Doc -}}
// {{.}}
{{end -}}
type {{.Node.Name}} struct { Client *{{.G.Capnp}}.Client }
type {{.Node.Name}} struct { Client {{.G.Capnp}}.Client }

{{ template "_typeid" .Node }}

Expand Down
Loading