Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+---
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  closed
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+---
Changes (by cohosh):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Deployed at broker at `2019/12/05 16:16:05`

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+-
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  merge_ready
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+-

Comment (by cohosh):

 Merged in `06298eec73`: https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/commit/?id=06298eec730aa2664bb61d4cce4ef56dfce91ee3

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+-
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  merge_ready
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+-
Changes (by phw):

 * status:  needs_review => merge_ready


Comment:

 Looks good to me!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+--

Comment (by cohosh):

 Added another commit and tested it locally. How does it look?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+--

Comment (by phw):

 This looks good. I left a review on your GitHub pull request. I had a
 minor suggestion and a question regarding the scope of what needs to be
 protected by a lock.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:  1
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+--
Changes (by cohosh):

 * actualpoints:   => 1


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  phw  |Sponsor:  Sponsor28
-+--
Changes (by cohosh):

 * reviewer:   => phw


Comment:

 Alright I modified the patch and added a test that produce a data race
 before the patch but pass afterwards. This is definitely ready for a
 review.

 For a broker update: the broker hasn't crashed for over 2 days so we have
 some recent metrics:
 https://metrics.torproject.org/collector/recent/snowflakes/

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:  Sponsor28
-+--

Comment (by cohosh):

 Setting `export GOMAXPROCS=1` doesn't seem to have helped. The broker
 crashed again at `2019/11/23 08:24:18` this morning.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:  Sponsor28
-+--

Comment (by cohosh):

 Replying to [comment:7 dcf]:
 > Perhaps because the old broker had 1 CPU core and the new one has 2 CPU
 cores? I can imagine that might affect goroutine scheduling which might
 affect the visibility of race conditions.
 Ah that would do it :) The reason we didn't notice then was because
 clients and proxies weren't around to trigger the race until we switched
 over the DNS records for the bamsoftware.com domains.
 >
 > You might try setting `GOMAXPROCS=1` in /etc/service/snowflake-
 broker/run as a temporary workaround, if indeed it helps.
 Okay I did this and restarted the broker as of `2019/11/22 22:50:27` that
 should work until we get this patch reviewed. Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+--
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  needs_review
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:  Sponsor28
-+--
Changes (by cohosh):

 * status:  assigned => needs_review


Comment:

 This race condition occurs because both the proxy process and the client
 process try to `Pop`/`Remove` the same snowflake from the heap.

 When a proxy polls the broker, it starts a go routine that waits for an
 offer through the snowflake's offer channel, or waits for a timeout:
 {{{
 go func(request *ProxyPoll) {
 select {
 case offer := <-snowflake.offerChannel:
 request.offerChannel <- offer
 case <-time.After(time.Second * ProxyTimeout):
 // This snowflake is no longer available to serve clients.
 // TODO: Fix race using a delete channel
 heap.Remove(ctx.snowflakes, snowflake.index)
 delete(ctx.idToSnowflake, snowflake.id)
 request.offerChannel <- nil
 }
 }(request)
 }}}

 Snowflakes are removed from the heap when they time out or if they are
 popped off the heap by a client:
 {{{
 func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r
 *http.Request) {

 ...

 // Delete must be deferred in order to correctly process answer
 request later.
 snowflake := heap.Pop(ctx.snowflakes).(*Snowflake)
 defer delete(ctx.idToSnowflake, snowflake.id)
 snowflake.offerChannel <- offer
 }}}

 A race can always occur where the timeout happens after the `Pop` removes
 the snowflake but before the offer is sent through `offerChannel`. This
 can be fixed through the use of locks and a check to see if
 `snowflake.index` has been set to `-1` (which happens after it has been
 popped off the heap). Here's a patch that adds synchronization to the
 broker to prevent simultaneous access to the heap as well as the
 `idToSnowflake` map: https://github.com/cohosh/snowflake/pull/14

 I'd like to do some tests before merging this to make sure that the
 synchronization doesn't slow things down too much, but a code review would
 be good at this point.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+---
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  assigned
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:  Sponsor28
-+---

Comment (by dcf):

 Replying to [comment:4 cohosh]:
 > The weird thing is that the #29207 update was deployed on the old broker
 at the same time but hasn't caused any problems. It's possible this was
 due to the DNS update to point to the new broker, but as seen from the
 metrics data, the old broker was still getting a lot of proxy polls for
 several days after.

 Perhaps because the old broker had 1 CPU core and the new one has 2 CPU
 cores? I can imagine that might affect goroutine scheduling which might
 affect the visibility of race conditions.

 You might try setting `GOMAXPROCS=1` in /etc/service/snowflake-broker/run
 as a temporary workaround, if indeed it helps.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32576 [Circumvention/Snowflake]: Fix race condition in snowflake broker (was: Snowflake broker not serving metrics correctly)

[Tor Bug Tracker & Wiki]

#32576: Fix race condition in snowflake broker
-+---
 Reporter:  cohosh   |  Owner:  cohosh
 Type:  defect   | Status:  assigned
 Priority:  Very High|  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:  metrics  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:  Sponsor28
-+---

Old description:

> Looks like the snowflake broker isn't serving metrics since 2019-11-14
> (which happens to coincide with the host migration.
>
> At first I thought this was a DNS issue and filed #32570, but even when I
> manually fetch https://snowflake-broker.torproject.net/metrics I'm
> getting outdated metrics.
>
> I've attached the result of the above fetch as well as the metrics file
> located on the snowflake broker host that the broker *should* be reading
> from.

New description:

 There is a race condition with the snowflake heap that has been causing
 the broker to crash several times a day. This race condition has existed
 in the broker for several years, but some recent updates as well as the
 host migration managed to shake it loose.

 

 This race condition is causing the snowflake broker to crash repeatedly
 and often since the migration. We noticed because CollecTor stopped
 collecting metrics since the restart on 14 November 2019.

--

Comment (by cohosh):

 Just updating the summary of this ticket to reflect the actual problem.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs