fix(client): flush the OS' DNS cache whenever resources change #5700
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Terraform Cloud Plan Output |
Performance Test ResultsTCP
UDP
|
| | InternalServerMsg::Ipc(IpcServerMsg::OnUpdateResources(_)) => { | ||
| // On every resources update, flush DNS to mitigate <https://github.com/firezone/firezone/issues/5052> | ||
| dns_controller.flush()?; | ||
| } |
There was a problem hiding this comment.
The callback handler code here is duplicated between the IPC service and the headless Client. Since they do almost the same thing, we can probably refactor it someday so that they share the same main loop, and the only difference is that one listens for IPC events and one listens for in-process commands
|
I think this may also be the cause of #4834. Linking it here so we have a thread to investigate when we can tackle that issue. Unfortunately from preliminary Googling I don't know if we have a button to push there for Android. Will need to test if this is also an issue on macOS / iOS but I don't think it is. I believe the OS either doesn't use a cache or flushes it for us. |
|
Yeah on macOS I did not get it to replicate. But that's strange because ChatGPT says macOS has a cache, and the fix in this PR is to flush when our resources change, which the OS shouldn't be aware of. So why doesn't it happen on other OSes? |
PR #5700 had a typo in it. I didn't notice that these match arms use `|`, so I accidentally flush the DNS for an event that doesn't need it. Only `OnUpdateResources` should flush DNS.
It's possible macOS does not have a cache before the DNS servers for VPN interfaces, to prevent this sort of issue. I assume Windows and Linux do cache before queries reach the tunnel interface, and possibly Android too. |
PR #5700 had a typo in it. I didn't notice that these match arms use `|`, so I accidentally flush the DNS for an event that doesn't need it. Only `OnUpdateResources` should flush DNS.
Closes #5052
On my dev VMs:
I tested with the headless Clients on Linux and Windows and it fixes the issue. On Windows I didn't replicate the issue with the GUI Client, on Linux this patch also fixes it for the GUI Client.