Re: [tor-bugs] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
-+
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  review-group-31  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  dgoulet  |Sponsor:
-+
Changes (by nickm):

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


Comment:

 squashed and merged; 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] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
-+
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:  merge_ready
 Priority:  Medium   |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  review-group-31  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  dgoulet  |Sponsor:
-+
Changes (by dgoulet):

 * status:  needs_review => merge_ready


Comment:

 Both fixups are very nice!

 Thanks. ACK.

--
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] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
-+
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:  needs_review
 Priority:  Medium   |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  review-group-31  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  dgoulet  |Sponsor:
-+
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 Okay; I've tried to resolve those.  Better now?

--
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] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
-+
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:  needs_revision
 Priority:  Medium   |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  review-group-31  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  dgoulet  |Sponsor:
-+
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 * Nitpick: I would wrap this in its own function like the init and cleanup
 functions:

 {{{
 +  {
 +int warning_severity = options->ProtocolWarnings ? LOG_WARN :
 LOG_INFO;
 +atomic_counter_exchange(_warning_severity_level,
 +warning_severity);
 +  }
 }}}

  Like a "set" function that wraps the `exchange()` so we have one function
 that is allow to set `protocol_warning_severity_level` so
 `init_protocol_warning_severity_level()` can also use it.

 * Quick thing also, I would document the reason why we use an atomic
 counter for that value that is at least explaining what the possible race
 is exactly:

 {{{
 +static atomic_counter_t protocol_warning_severity_level;
 }}}

--
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] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  assigned => needs_review


--
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] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  defect| Status:  assigned
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by nickm):

 Branch `bug23954` is ready for review.  I used the atomic_counter_t
 structure here so we could get good performance on platforms with
 stdatomic support.

--
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

[tor-bugs] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN

[Tor Bug Tracker & Wiki]

#23954: Race condition in LOG_PROTOCOL_WARN
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  defect| Status:  assigned
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal|   Keywords:
Actual Points:|  Parent ID:
   Points:|   Reviewer:
  Sponsor:|
--+
 LOG_PROTOCOL_WARN currently calls get_options().  After #23952 is merged,
 it will call get_protocol_warning_severity_level().  The trouble is, both
 of these functions access a global variable without acquiring a lock...
 and LOG_PROTOCOL_WARN can be invoked from outside the main thread.

 Let's fix this once #23952 and #23953 are merged.

--
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