Opened 4 months ago

Last modified 9 days ago

#28226 needs_review enhancement

Backend for an async publish-subcribe messaging system for cross-module communications

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-pubsub, sponsor31-maybe, network-team-roadmap-2019-Q1Q2
Cc: Actual Points:
Parent ID: Points: 8
Reviewer: catalyst Sponsor: Sponsor31-must

Description

In Mexico City I walked with Teor and Catalyst about a generic publish/subscribe system. Thanks for feedback from the network team, I have an initial API I like, and I've tried to do an implementation.

Child Tickets

Change History (14)

comment:1 Changed 4 months ago by nickm

Status: assignedneeds_review

See branch messaging in my public repository, with PR at https://github.com/torproject/tor/pull/452

comment:2 Changed 4 months ago by nickm

Next steps for me here:

  • send the design document to tor-dev
  • Write the API for testing modules that use this
  • write the subsystem implementation.

comment:3 Changed 4 months ago by dgoulet

Keywords: tor-pubsub added
Reviewer: catalyst

(Declaring this new subsystem "tor-pubsub" :)

comment:4 Changed 4 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:5 Changed 4 months ago by nickm

I've pushed a few more commits based on your comments, to try to improve naming and documentation.

comment:6 Changed 6 weeks ago by gaba

Keywords: s31-maybe added
Sponsor: Sponsor8

comment:7 Changed 6 weeks ago by gaba

Keywords: sponsor31-maybe added; s31-maybe removed

comment:8 Changed 6 weeks ago by nickm

I've made a new branch as messaging_v2, with a PR at https://github.com/torproject/tor/pull/648.

It refactors the old branch considerably, and incorporates a bunch of your suggestions.

(It does not switch from using "auxdata" to using a subtype of "msg_t" for additional data as we had suggested, since I am not sure about that yet. We can do that part later if need be.)

comment:9 Changed 6 weeks ago by nickm

Milestone: Tor: 0.4.0.x-finalTor: 0.4.1.x-final

comment:10 Changed 4 weeks ago by nickm

Sponsor: Sponsor31-can

comment:11 Changed 4 weeks ago by nickm

Note from Catalyst: In "Add MESG as a new log domain.", N_LOGGING_DOMAINS needs to be incremented.

comment:12 Changed 4 weeks ago by catalyst

Partial review so far, by commit:

b7358da11 namemap
seems ok. I have a few misgivings about the type punning used for the temporary object, but it might be safe
d26a213c9 LD_MESG
seems ok, except for needing to bump the total as mentioned above
749456940 semicolon eater
seems ok. Maybe we want to uses this in ht.h, but that means changing all its users, so maybe not in this ticket
33c20c70d smartlist_grow
seems ok, assuming the sizes bounds check in smartlist_ensure_capacity() is correct, which from a quick glance it seems to be

comment:13 Changed 11 days ago by gaba

Keywords: network-team-roadmap-2019-Q1Q2 added
Points: 8
Sponsor: Sponsor31-canSponsor31-must

comment:14 Changed 9 days ago by catalyst

Made some comments on github; still reviewing.

Note: See TracTickets for help on using tickets.