Re: [tor-bugs] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  refactoring, technical-debt, |  implemented
  review-group-31, 034-triage-20180328,  |  Actual Points:
  034-included-20180401  |
Parent ID:  #25500   | Points:
 Reviewer:  dgoulet  |Sponsor:
 |  Sponsor8
-+-
Changes (by nickm):

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


Comment:

 Travis seems okay with it.  Squashed and 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

Re: [tor-bugs] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  enhancement  | Status:
 |  merge_ready
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  refactoring, technical-debt, |  Actual Points:
  review-group-31, 034-triage-20180328,  |
  034-included-20180401  |
Parent ID:  #25500   | Points:
 Reviewer:  dgoulet  |Sponsor:
 |  Sponsor8
-+-
Changes (by dgoulet):

 * status:  needs_review => merge_ready


Comment:

 Thanks. Changes lgtm! Not sure if it passes the CI but it passes make test
 I'm happy with the fixes.

--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  enhancement  | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  refactoring, technical-debt, |  Actual Points:
  review-group-31, 034-triage-20180328,  |
  034-included-20180401  |
Parent ID:  #25500   | Points:
 Reviewer:  dgoulet  |Sponsor:
 |  Sponsor8
-+-
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 Okay, I've made the requested changes!

 First I had to rebase the branch, since there were several incompatible
 changes to master since this branch began.  Now it is in a new branch
 called `isolate_libevent_2`.

 I made the changes you requested, except for this:

 > This function threadpool_register_reply_event() doesn't use the nice
 mainloop_event_t API

 The problem here is that mainloop_event_t is only for timed events or
 events that we trigger via event_active(): we can't use it for events that
 trigger via a socket.

--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  refactoring, technical-debt, |  Actual Points:
  review-group-31, 034-triage-20180328,  |
  034-included-20180401  |
Parent ID:  #25500   | Points:
 Reviewer:  dgoulet  |Sponsor:
 |  Sponsor8
-+-
Changes (by nickm):

 * keywords:
 refactoring, technical-debt, review-group-31, 034-triage-20180328,
 034-removed-20180328
 =>
 refactoring, technical-debt, review-group-31, 034-triage-20180328,
 034-included-20180401
 * sponsor:   => Sponsor8
 * parent:   => #25500


Comment:

 This is part of #25500, since it will help with some of our other mainloop
 improvements.

--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  refactoring, technical-debt, |  Actual Points:
  review-group-31|
Parent ID:   | Points:
 Reviewer:  dgoulet  |Sponsor:
-+-
Changes (by dgoulet):

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


Comment:

 From commit `6c5a4bef4e77cf37`:

 * I would propose that `mainloop_event_new()` validates at least the `cb`
 argument passed with a `tor_assert()` (seems it can't be NULL nor it makes
 sense to be NULL).

  I would also document the function that the `userdata` ownership is
 passed to the returned `mainloop_event_`.

 * Can we use `mainloop_event_activate()` if the event was never
 scheduled/added to the main loop?

 * Can I use `tv = NULL` in `mainloop_event_schedule()` to tell it "now" or
 `tv` must be a valid pointer?

 * I would document what this does if called multiple time:
 `mainloop_event_cancel()`.

 * Maybe we should have this using the `FREE_AND_NULL` macro scheme?
 `mainloop_event_free()`

 From commit `3edf64a95f91d9be`:

 * This function `threadpool_register_reply_event()` doesn't use the nice
 `mainloop_event_t` API and I can see that is because we create an event
 with specific flags (EV_PERSIST | EV_READ).

  And then it goes on using `event_add()` directly. Couldn't we remove all
 this and make it that it would use that mainloop interface created prior?

 * Also, why is `threadpool_register_reply_event()` is taking a base event,
 can't we abstract that to always use `tor_libevent_get_base()` or that
 doesn't play nice in multi thread?

--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
+--
 Reporter:  nickm   |  Owner:  nickm
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:  Tor:
|  0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  refactoring technical-debt  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  |Sponsor:
+--
Changes (by nickm):

 * status:  assigned => needs_review
 * milestone:  Tor: 0.3.3.x-final => Tor: 0.3.4.x-final


--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
+--
 Reporter:  nickm   |  Owner:  nickm
 Type:  enhancement | Status:  assigned
 Priority:  Medium  |  Milestone:  Tor:
|  0.3.3.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  refactoring technical-debt  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  |Sponsor:
+--

Comment (by nickm):

 I've started some work here as `isolate_libevent`.  Let's take a look and
 maybe merge it when 0.3.3 rolls around.

--
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] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

[Tor Bug Tracker & Wiki]

#23750: Isolate libevent usage to a few locations
--+
 Reporter:  nickm |  Owner:  nickm
 Type:  enhancement   | Status:  assigned
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal|   Keywords:  refactoring technical-debt
Actual Points:|  Parent ID:
   Points:|   Reviewer:
  Sponsor:|
--+
 With #21841, we restricted openssl header usage to a small number of
 modules.  We should do the same with libevent.

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