Skip to content

Connect: Add mutex to gateway.Gateway#18341

Closed
ravicious wants to merge 4 commits intomasterfrom
ravicious/gateway-mutex
Closed

Connect: Add mutex to gateway.Gateway#18341
ravicious wants to merge 4 commits intomasterfrom
ravicious/gateway-mutex

Conversation

@ravicious
Copy link
Copy Markdown
Member

No description provided.

That was a weird name. I think it was originally meant to be something
else, then I did search and replace in this file and it change it to
that name.
This will help us in tests in the upcoming commits.
daemon.Service.ListGateways used to copy each gateway by dereferencing
a pointer and then returning a new slice with all gateways. This was done
to protect from callsites mutating the original slice with gateways
without obtaining a lock on daemon.Service.mu.

However, methods such as `daemon.Service.CreateGateway` still returned
a pointer to the gateway, technically allowing other callsites to modify
the gateway without going through daemon.Service methods.

Since in the previous commit we added a mutex to gateway.Gateway, we can
no longer copy the struct by value. To work around this problem, we're
going to return protos from daemon.Service methods which previously
returned gateway pointers.

Callsites that use those methods don't need real gateways anyway, they
just need to read a couple of fields from them to return a response from
the API endpoint.
@ravicious ravicious force-pushed the ravicious/gateway-mutex branch from dca5ae6 to 479d772 Compare November 29, 2022 13:03
@ravicious
Copy link
Copy Markdown
Member Author

The mutex was initially added in #18259 to guard the certs when calling Gateway.ReloadCert. Then I reverted this change within the same PR when I realized that

  • We don't need a mutex there as the gateway processes connections serially so lib/teleterm's code will never cause two concurrent calls to Gateway.ReloadCert.
  • Gavin wanted to add a mutex to LocalProxy itself.
  • …and I figured out that adding the mutex is more complicated due to reasons described in 479d772.

Ultimately, at that stage the mutex was something nice to have rather than something which would actually prevent bugs in the code – any operations on the list of gateways are protected by a mutex on the daemon level anyway.

I'm going to close this PR for now.

@ravicious ravicious closed this Mar 1, 2023
@ravicious ravicious deleted the ravicious/gateway-mutex branch March 1, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant