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

CreatePermission is called for every TURN packet sent #77

Closed
Andrepuel opened this issue Jul 30, 2021 · 4 comments
Closed

CreatePermission is called for every TURN packet sent #77

Andrepuel opened this issue Jul 30, 2021 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@Andrepuel
Copy link
Contributor

From the source code

async fn send_to(&mut self, p: &[u8], addr: SocketAddr) -> Result<usize> {
    // ...
    let mut perm = if let Some(perm) = self.perm_map.find(&addr) {
        *perm
    } else {
        let perm = Permission::default();
        self.perm_map.insert(&addr, perm);
        perm
    };

    let mut result = Ok(());
    for _ in 0..MAX_RETRY_ATTEMPTS {
        result = self.create_perm(&mut perm, addr).await;
        if let Err(err) = &result {
            if !Error::ErrTryAgain.equal(err) {
                break;
            }
        }
    }

The perm variable is read from perm_map but never written back to it. So, every time send_to is called, it will get a Idle permission, which triggers a new CreatePermission on the create_perm function.

After the create_perm is called, the perm variable should be set back to perm_map.

Besides that, there is no Lifetime tracking. If we didn't receive any packet from a given address for more than 300 seconds, then the Permission for that given address should be removed from the perm_map. Because we must update the permission.

@rainliu rainliu self-assigned this Jul 30, 2021
@rainliu rainliu added the bug Something isn't working label Jul 30, 2021
@rainliu
Copy link
Member

rainliu commented Jul 30, 2021

@Andrepuel
Copy link
Contributor Author

Besides that, there is no Lifetime tracking. If we didn't receive any packet from a given address for more than 300 seconds, then the Permission for that given address should be removed from the perm_map. Because we must update the permission.

Could you confirm this? Upon closer inspection I've found a reference to a timed refresh of permissions:

https://sourcegraph.com/github.com/webrtc-rs/turn@909eac74d669d62202cd89783ef7e6136e1f4026/-/blob/src/client/relay_conn.rs?L600

Does that mean we don't need to track permissions timeout?

@Andrepuel Andrepuel changed the title CreatePermission is called for every TURN packet send CreatePermission is called for every TURN packet sent Jul 30, 2021
@rainliu
Copy link
Member

rainliu commented Jul 31, 2021

The bug of CreatePermission is called for every TURN packet sent should be fixed in commit # f593cc4d1940a9cbae03ff32d70b33d2f8bda969.

@Andrepuel
Copy link
Contributor Author

The bug of CreatePermission is called for every TURN packet sent should be fixed in commit # f593cc4d1940a9cbae03ff32d70b33d2f8bda969.

I can confirm that this commit fixes the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants