Skip to content

Commit

Permalink
call messages: let the transport release the cap table
Browse files Browse the repository at this point in the history
For two reasons:

1. As clarified in #308, the transport will release these when the
   message is freed, so we don't need to do this here to avoid a leak,
   and letting the transport deal with it is simpler.
2. I smell a race condition: this releases the clients before the
   message is actually on the wire. I *think* it is actually fine,
   because I think by this time the message is already in the queue and
   so is morally on the wire, but it's a bit harder to reason about,
   and makes me nervous.

I *believe* there is no functional change here.
  • Loading branch information
zenhack committed Sep 30, 2022
1 parent eb4e490 commit c683146
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 10 deletions.
5 changes: 0 additions & 5 deletions rpc/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,13 @@ func (c *Conn) newImportCallMessage(msg rpccp.Message, imp importID, qid questio
}
m := args.Message()
if err := s.PlaceArgs(args); err != nil {
for _, c := range m.CapTable {
c.Release()
}
m.CapTable = nil
return rpcerr.Failedf("place arguments: %w", err)
}
clients := m.CapTable
syncutil.With(&c.mu, func() {
// TODO(soon): save param refs
_, err = c.fillPayloadCapTable(payload, clients)
})
releaseList(clients).release()
if err != nil {
return rpcerr.Annotatef(err, "build call message")
}
Expand Down
5 changes: 0 additions & 5 deletions rpc/question.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,18 +232,13 @@ func (c *Conn) newPipelineCallMessage(msg rpccp.Message, tgt questionID, transfo
}
m := args.Message()
if err := s.PlaceArgs(args); err != nil {
for _, c := range m.CapTable {
c.Release()
}
m.CapTable = nil
return rpcerr.Failedf("place arguments: %w", err)
}
clients := m.CapTable
syncutil.With(&c.mu, func() {
// TODO(soon): save param refs
_, err = c.fillPayloadCapTable(payload, clients)
})
releaseList(clients).release()

if err != nil {
return rpcerr.Annotatef(err, "build call message")
Expand Down

0 comments on commit c683146

Please sign in to comment.