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.

Sign up for GitHub

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

Jump to bottom

Remove remaining non-hierarchical locking in the rpc package. #406

Merged
merged 8 commits into from
Dec 29, 2022
Merged

Remove remaining non-hierarchical locking in the rpc package. #406

merged 8 commits into from
Dec 29, 2022

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented Dec 28, 2022

Builds on #403, which should be merged first.

This removes all remaining unlock uses that aren't either:

Lock()
defer Unlock()

or syncutil.With.

@zenhack
Make use of syncutil.With
6cadf4e
@zenhack
Minor cleanup
5617569 
Giving this a name makes it easier to follow what's going on here.
@zenhack
Factor out construction of Recv.
28f46de
@zenhack
Centralize remaining unlock in handleCall
8aa94d5 
I don't love this, but at least it gets us to a place where the locking
is reasonably simple.
@zenhack
Minor stylistic change.
6da1fd2 
Per Louis's suggestion
@zenhack
Merge branch 'handleCall-locking' into HEAD
36b888f
@zenhack
handleReturn: hierarchical locking
067def2
@zenhack
handleDisembargo: hierarchical locking.
96b6211 
This is the last place in the rpc package where we're unlocking c.lk,
and it isn't either:

- using synctuil.With()
- in a defer statement, immediately after the Lock().
@lthibault
Copy link
Collaborator 

ok  	capnproto.org/go/capnp/v3/rpc	1.201s
panic: close of closed channel

goroutine 200 [running]:
capnproto.org/go/capnp/v3.Client.Release({0x9856870})
	/home/runner/work/go-capnproto2/go-capnproto2/capability.go:587 +0xe6
capnproto.org/go/capnp/v3.(*Message).Reset(0x993e120, {0x0, 0x0})
	/home/runner/work/go-capnproto2/go-capnproto2/message.go:139 +0xc6
capnproto.org/go/capnp/v3/rpc/transport.(*transport).RecvMessage.func1()
	/home/runner/work/go-capnproto2/go-capnproto2/rpc/transport/transport.go:171 +0x3b
capnproto.org/go/capnp/v3/rpc.(*Conn).handleReturn.func2()
	/home/runner/work/go-capnproto2/go-capnproto2/rpc/rpc.go:1008 +0xd3
created by capnproto.org/go/capnp/v3/rpc.(*Conn).handleReturn
	/home/runner/work/go-capnproto2/go-capnproto2/rpc/rpc.go:1001 +0x7e5
exit status 2
FAIL	capnproto.org/go/capnp/v3/rpc	0.822s

I think this is a new one.

@zenhack
Copy link
Contributor Author

zenhack commented Dec 28, 2022

That looks like #348

@zenhack zenhack merged commit 953da2c into capnproto:main Dec 29, 2022
@zenhack zenhack deleted the nh-locking branch December 29, 2022 07:16
@lthibault lthibault mentioned this pull request Jan 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lthibault lthibault lthibault approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zenhack @lthibault