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

Migrate valyala/tcplisten to this repo #1926 #1929

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ksw2000
Copy link
Contributor

@ksw2000 ksw2000 commented Dec 19, 2024

Resolve #1926

This PR moves the valyala/tcplisten package to fasthttp and also migrates PR#6 and PR#10 together. Additionally, it upgrades the deprecated ioutil package and replaces the +build directive with go:build. Furthermore, it fixes the warning in tcplisten_test.go regarding "the goroutine calls T.Fatalf, which must be called in the same goroutine as the test".

@ksw2000
Copy link
Contributor Author

ksw2000 commented Dec 19, 2024

I think the error is caused by DeferAccept and FastOpen not being implemented in darwin 🤔

@ksw2000 ksw2000 marked this pull request as ready for review December 20, 2024 07:26
@gaby
Copy link
Contributor

gaby commented Dec 24, 2024

@ksw2000 Can these be added too?

@ksw2000
Copy link
Contributor Author

ksw2000 commented Dec 29, 2024

@gaby

@ksw2000 Can these be added too?

About implementation of the keepalive feature:

When calling NewListener for tcplisten.Config, it internally calls net.FileListener to generate a net.Listener struct/interface, which contains the private field lc net.ListenConfig. I don't have a graceful solution to implement keepalive at the moment.

However, there is an alternative method to set keepAlive after calling ln.Accept().

func main() {
	cfg := tcplisten.Config{
		ReusePort: true,
	}
	ln, err := cfg.NewListener("tcp", "0.0.0.0:8082")
	if err != nil {
		panic(err)
	}
	for {
		conn, err := ln.Accept()
		if err != nil {
			panic(err)
		}
		if conn, ok := conn.(*net.TCPConn); ok {
			conn.SetKeepAlive(true)
			conn.SetKeepAlivePeriod(time.Second)
		}
		go func() {
			defer conn.Close()
			fmt.Println("Accepted connection from", conn.RemoteAddr())
		}()
	}
}

Of course, if we still want to add the keepAlive feature, we could also wrap the net.Listener generated by net.FileListener and override the Accept() method, set keepalive before returning conn. But, in my opinion, this approach might confuse the original usage patterns.

@gaby
Copy link
Contributor

gaby commented Dec 29, 2024

@ksw2000 Agree, it does sound confusing.

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.

Migrate tcplisten to this repo
2 participants