[tor-bugs] #33367 [Circumvention/Snowflake]: Snowflake server using 1.5 GB memory, preventing other allocations

[Tor Bug Tracker & Wiki]

#33367: Snowflake server using 1.5 GB memory, preventing other allocations
-------------------------------------+------------------------------
 Reporter:  dcf                      |          Owner:  (none)
     Type:  defect                   |         Status:  needs_review
 Priority:  Medium                   |      Milestone:
Component:  Circumvention/Snowflake  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------------
Changes (by dcf):

 * status:  new => needs_review


Comment:

 Replying to [comment:2 dcf]:
 > Initially I suspected the recent websocketconn changes from #33144, but
 those can only have had effect since 2020-02-10 18:57 when the server was
 restarted, and the earliest reports of "Could not connect to the bridge"
 predate that

 Well, there is a leak in the new websocketconn code anyway. You can see it
 with this patch. The `"Close pw1"` line runs but the `"Close pr2"` line
 does not, leaking a goroutine.
 {{{#!diff
 diff --git a/common/websocketconn/websocketconn.go
 b/common/websocketconn/websocketconn.go
 index 46bb977..0fb7fa9 100644
 --- a/common/websocketconn/websocketconn.go
 +++ b/common/websocketconn/websocketconn.go
 @@ -3,4 +3,5 @@ package websocketconn
  import (
         "io"
 +       "log"
         "time"

 @@ -106,8 +107,10 @@ func New(ws *websocket.Conn) *Conn {
         go func() {
                 pw1.CloseWithError(closeErrorToEOF(readLoop(pw1, ws)))
 +               log.Printf("%p Close pw1", ws)
         }()
         pr2, pw2 := io.Pipe()
         go func() {
                 pr2.CloseWithError(closeErrorToEOF(writeLoop(ws, pr2)))
 +               log.Printf("%p Close pr2", ws)
         }()
         return &Conn{
 }}}

 https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/tree/common/websocketconn/websocketconn.go?id=ca9ae12c383405bc9a755e1bc902e9755495c1f1#n106
 {{{#!go
         pr1, pw1 := io.Pipe()
         go func() {
                 pw1.CloseWithError(closeErrorToEOF(readLoop(pw1, ws)))
         }()
         pr2, pw2 := io.Pipe()
         go func() {
                 pr2.CloseWithError(closeErrorToEOF(writeLoop(ws, pr2)))
         }()
         return &Conn{
                 Conn:   ws,
                 Reader: pr1,
                 Writer: pw2,
         }
 }}}

 The first goroutine reads messages from the WebSocket `ws` and writes them
 to `pw1`, which causes them to be returned from the `Conn`'s `Read` method
 (`pr1` is the `Reader` and is the other end of the `pw1` pipe). This part
 is fine.

 The second goroutine reads from `pr2` and writes to the WebSocket `ws`.
 `pr2` gets its input from things written using the `Conn`'s `Write`
 method, which feeds directly into `pw2`.

 Problem is, there's nothing that ever closes `pw2`. The `Close` method
 closes the WebSocket, so if the second goroutine ever ''were'' to try to
 write to it, it would detect an error and exit. But as long as nothing
 further ever calls `Write`, nothing is written to `pw2` and so the
 goroutine waits forever.

 Here's a patch that closes the internal `Pipe`s when the `Close` method is
 called, and a test case that fails before the patch and does not fail
 after the patch.
 https://gitweb.torproject.org/user/dcf/snowflake.git/commit/?h=bug33367-websocketconn&id=380b133155ad725126bc418d0e66b3c550b4c555

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33367#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online