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

Suggestion: Decouple Header Reading From net.Conn #111

Open
adrianosela opened this issue Jul 6, 2024 · 2 comments
Open

Suggestion: Decouple Header Reading From net.Conn #111

adrianosela opened this issue Jul 6, 2024 · 2 comments

Comments

@adrianosela
Copy link

Having a sync.Once be checked in each net.Conn operation (i.e. on every Read()) seems quite inefficient. I suggest enqueuing accepted connections from the inner net.Listener's Accept(), processing/reading the header asynchronously, and having the publicly exposed net.Listener return the already-processed net.Conns...

Here's some pseudocode to illustrate what I mean:

import "net"

type listener struct {
	inner net.Listener

	in  chan net.Conn
	out chan net.Conn
}

// function that reads the proxy protocol header and returns a net.Conn
// implementation which returns the right address on RemoteAddr()
func processFunc(in net.Conn) net.Conn {
    // assume implemented
}

func NewListener(inner net.Listener) net.Listener {
	wrapped := &listener{
		inner: inner,
		in:    make(chan net.Conn, 1000),
		out:   make(chan net.Conn, 1000),
	}

	// kick-off go routine to process newly accepted connections
	go func() {
		for conn := range wrapped.in {
			conn := conn // avoid address capture
			go func() {
				processed := processFunc(conn)
				if processed != nil {
					wrapped.out <- processed
				}
			}()
		}
	}()

	// kick-off go routine to accept new connections
	go func() {
		for {
			conn, err := inner.Accept()
			if err != nil {
				return // accept errors are not recoverable
			}
			wrapped.in <- conn
		}
	}()

	return wrapped
}

func (l *listener) Accept() (net.Conn, error) {
	return <-l.out, nil
}

func (l *listener) Close() error {
	return l.inner.Close()
}

func (l *listener) Addr() net.Addr {
	return l.inner.Addr()
}

If there's interest in this I would love to submit a PR... seems indeed more efficient!

@AlexanderYastrebov
Copy link

Having a sync.Once be checked in each net.Conn operation

You may want to compose a benchmark (or read stdlib sources) to find out that sync.Once is as fast as reading atomic integer.

@pires
Copy link
Owner

pires commented Jan 15, 2025

The benchmark proposal is solid.

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

No branches or pull requests

3 participants