Skip to content

Conversation

@ibigbug
Copy link
Member

@ibigbug ibigbug commented Feb 18, 2025

🤔 This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Changelog is provided or not needed

@Itsusinn
Copy link
Member

Itsusinn commented Feb 23, 2025

@Watfaq/clash-rs-devs
There is actually another solution for global states (axum is using something similiar).

Define a struct named AppConext where we store states that maybe global
And a static var for global useage

struct AppConext {
    mark: u64
    name: String
    tun: tunState
}
static GLOBAL_CONTEXT: ArcSwapOption<AppConext> = todo!()
// or ...
static GLOBAL_CONTEXT: RwLock<AppConext> = todo!()

// if we want more fine-grained lock
struct AppConext {
    mark: AtomicU64
    name: ArcSwap<String>
    other: RwLock<AnotherState>
}
static GLOBAL_CONTEXT: Lazy<AppConext> = todo!()

Where we use global states:

fn apply_socket_opts(){
    apply_socket_opts_with_ctx(GLOBAL_CONTEXT.get())
}
fn apply_socket_opts_with_ctx(ctx: AppContext){
    todo!()
}

when we use *_with_ctx funtcions ?

  • a. when define another with_ctx function
fn bind_interface(){
    bind_interface_with_ctx(GLOBAL_CONTEXT.get())
}
fn bind_interface_with_ctx(ctx: AppContext){
    todo!()
}
// we must caller's Context
  • b. when we writing parallel tests
#[test]
fn test_1(){
    GLOBAL_CONTEXT.init(AppConext { mark:41 })
    apply_socket_opts()
}
#[test]
fn test_2(){
    GLOBAL_CONTEXT.init(AppConext { mark:34 })
    apply_socket_opts()
}
// that may cause
// 1. panic due to multi init if we use lazy
// 2. actual GLOBAL_CONTEXT not clear, `mark` maybe 34 in test_1 or 41 in test_2


#[test]
fn test_1(){
    let ctx = AppConext { mark:41 }
    apply_socket_opts_with_ctx(ctx)
}
#[test]
fn test_2(){
    let ctx = AppConext { mark:34 }
    apply_socket_opts_with_ctx(ctx)
}

when we use funtcions without _with_ctx ?

Actually saying... I dont know, we could pass ctx everywhere

fn main(){
    let ctx = AppContext {
        ..
    }
    func_1_ctx(ctx);
    func_2_ctx(ctx);
}
fn func_1_with_ctx(ctx:Context){
}
fn func_2_with_ctx(ctx:Context){
}

or we dont use ctx only in top-level?

fn main(){
    let ctx = AppContext {
        ..
    }
    GLOBAL_CONTEXT.init(ctx);
    func_1();
    func_2();
}

@ibigbug
Copy link
Member Author

ibigbug commented Feb 23, 2025

Sure. That's def a more elegant solution.

There's probably some issues with the current tun udp handling concurrent reqs that I'm still trying to figure out.

@Itsusinn
Copy link
Member

There's probably some issues with the current tun udp handling concurrent reqs that I'm still trying to figure out.

TUN UDP + FAKE-IP + DNS hijack ?

@ibigbug
Copy link
Member Author

ibigbug commented Feb 23, 2025

Likely. Haven't test other scenarios tho.

@Itsusinn
Copy link
Member

On android TUN + dns-hijack works well (master branch). It might be route problems?

@ibigbug ibigbug changed the title WIP feat(dns): enhance dns hijack with fake IP enabled Feb 25, 2025
@ibigbug
Copy link
Member Author

ibigbug commented Feb 25, 2025

I'll make another PR to apply the global state refactor

@ibigbug ibigbug marked this pull request as ready for review February 25, 2025 07:51
@Itsusinn
Copy link
Member

I'll make another PR to apply the global state refactor

I think we can just pass context everywhere

@codecov
Copy link

codecov bot commented Feb 25, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@ibigbug ibigbug merged commit 8bc1ca7 into master Feb 25, 2025
31 of 32 checks passed
@ibigbug ibigbug deleted the dns-hijack branch February 25, 2025 16:14
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.

3 participants