TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
package quic
|
|
|
|
|
|
|
|
import (
|
2024-02-12 18:58:55 +00:00
|
|
|
"errors"
|
|
|
|
"net"
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
"sync"
|
2024-03-13 13:30:38 +00:00
|
|
|
"sync/atomic"
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
"time"
|
|
|
|
|
2023-05-06 00:42:41 +00:00
|
|
|
"github.com/quic-go/quic-go"
|
2024-02-12 18:58:55 +00:00
|
|
|
"github.com/rs/zerolog"
|
|
|
|
"github.com/rs/zerolog/log"
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
)
|
|
|
|
|
2024-03-05 15:07:59 +00:00
|
|
|
// The error that is throw by the writer when there is `no network activity`.
|
|
|
|
var idleTimeoutError = quic.IdleTimeoutError{}
|
|
|
|
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
type SafeStreamCloser struct {
|
2024-02-12 18:58:55 +00:00
|
|
|
lock sync.Mutex
|
|
|
|
stream quic.Stream
|
|
|
|
writeTimeout time.Duration
|
|
|
|
log *zerolog.Logger
|
2024-03-13 13:30:38 +00:00
|
|
|
closing atomic.Bool
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
}
|
|
|
|
|
2024-02-12 18:58:55 +00:00
|
|
|
func NewSafeStreamCloser(stream quic.Stream, writeTimeout time.Duration, log *zerolog.Logger) *SafeStreamCloser {
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
return &SafeStreamCloser{
|
2024-02-12 18:58:55 +00:00
|
|
|
stream: stream,
|
|
|
|
writeTimeout: writeTimeout,
|
|
|
|
log: log,
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
func (s *SafeStreamCloser) Read(p []byte) (n int, err error) {
|
|
|
|
return s.stream.Read(p)
|
|
|
|
}
|
|
|
|
|
|
|
|
func (s *SafeStreamCloser) Write(p []byte) (n int, err error) {
|
|
|
|
s.lock.Lock()
|
|
|
|
defer s.lock.Unlock()
|
2024-02-12 18:58:55 +00:00
|
|
|
if s.writeTimeout > 0 {
|
|
|
|
err = s.stream.SetWriteDeadline(time.Now().Add(s.writeTimeout))
|
|
|
|
if err != nil {
|
|
|
|
log.Err(err).Msg("Error setting write deadline for QUIC stream")
|
|
|
|
}
|
|
|
|
}
|
|
|
|
nBytes, err := s.stream.Write(p)
|
|
|
|
if err != nil {
|
2024-03-13 13:30:38 +00:00
|
|
|
s.handleWriteError(err)
|
2024-02-12 18:58:55 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
return nBytes, err
|
|
|
|
}
|
|
|
|
|
|
|
|
// Handles the timeout error in case it happened, by canceling the stream write.
|
2024-03-13 13:30:38 +00:00
|
|
|
func (s *SafeStreamCloser) handleWriteError(err error) {
|
|
|
|
// If we are closing the stream we just ignore any write error.
|
|
|
|
if s.closing.Load() {
|
|
|
|
return
|
|
|
|
}
|
2024-02-12 18:58:55 +00:00
|
|
|
var netErr net.Error
|
|
|
|
if errors.As(err, &netErr) {
|
|
|
|
if netErr.Timeout() {
|
2024-03-13 13:30:38 +00:00
|
|
|
// We don't need to log if what cause the timeout was no network activity.
|
2024-03-05 15:07:59 +00:00
|
|
|
if !errors.Is(netErr, &idleTimeoutError) {
|
|
|
|
s.log.Error().Err(netErr).Msg("Closing quic stream due to timeout while writing")
|
|
|
|
}
|
2024-03-13 13:30:38 +00:00
|
|
|
// We need to explicitly cancel the write so that it frees all buffers.
|
2024-02-12 18:58:55 +00:00
|
|
|
s.stream.CancelWrite(0)
|
|
|
|
}
|
|
|
|
}
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
}
|
|
|
|
|
|
|
|
func (s *SafeStreamCloser) Close() error {
|
2024-03-13 13:30:38 +00:00
|
|
|
// Set this stream to a closing state.
|
|
|
|
s.closing.Store(true)
|
|
|
|
|
TUN-5621: Correctly manage QUIC stream closing
Until this PR, we were naively closing the quic.Stream whenever
the callstack for handling the request (HTTP or TCP) finished.
However, our proxy handler may still be reading or writing from
the quic.Stream at that point, because we return the callstack if
either side finishes, but not necessarily both.
This is a problem for quic-go library because quic.Stream#Close
cannot be called concurrently with quic.Stream#Write
Furthermore, we also noticed that quic.Stream#Close does nothing
to do receiving stream (since, underneath, quic.Stream has 2 streams,
1 for each direction), thus leaking memory, as explained in:
https://github.com/lucas-clemente/quic-go/issues/3322
This PR addresses both problems by wrapping the quic.Stream that
is passed down to the proxying logic and handle all these concerns.
2022-01-27 22:37:45 +00:00
|
|
|
// Make sure a possible writer does not block the lock forever. We need it, so we can close the writer
|
|
|
|
// side of the stream safely.
|
|
|
|
_ = s.stream.SetWriteDeadline(time.Now())
|
|
|
|
|
|
|
|
// This lock is eventually acquired despite Write also acquiring it, because we set a deadline to writes.
|
|
|
|
s.lock.Lock()
|
|
|
|
defer s.lock.Unlock()
|
|
|
|
|
|
|
|
// We have to clean up the receiving stream ourselves since the Close in the bottom does not handle that.
|
|
|
|
s.stream.CancelRead(0)
|
|
|
|
return s.stream.Close()
|
|
|
|
}
|
2022-08-10 13:21:55 +00:00
|
|
|
|
|
|
|
func (s *SafeStreamCloser) CloseWrite() error {
|
|
|
|
s.lock.Lock()
|
|
|
|
defer s.lock.Unlock()
|
|
|
|
|
|
|
|
// As documented by the quic-go library, this doesn't actually close the entire stream.
|
|
|
|
// It prevents further writes, which in turn will result in an EOF signal being sent the other side of stream when
|
|
|
|
// reading.
|
|
|
|
// We can still read from this stream.
|
|
|
|
return s.stream.Close()
|
|
|
|
}
|
2023-04-27 18:49:56 +00:00
|
|
|
|
|
|
|
func (s *SafeStreamCloser) SetDeadline(deadline time.Time) error {
|
|
|
|
return s.stream.SetDeadline(deadline)
|
|
|
|
}
|