Messaging v3 merged: the big publish-subscribe branch of fun. #859

nmathewson · 2019-03-27T00:14:50Z

No description provided.

We'll be using this for four kinds of identifier in dispatch.c

You use this when you're defining a macro to be used at file scope, and you want to require a semicolon afterwards.

Tests included.

We already do this in our log_debug() macro, but there are times when we'd like to avoid allocating or precomputing something that we are only going to log if debugging is on.

This module implements a way to send messages from one module to another, with associated data types. It does not yet do anything to ensure that messages are correct, that types match, or that other forms of consistency are preserved.

This "publish/subscribe" layer sits on top of lib/dispatch, and tries to provide more type-safety and cross-checking for the lower-level layer. Even with this commit, we're still not done: more checking will come in the next commit, and a set of usability/typesafety macros will come after.

This code tries to prevent a large number of possible errors by enforcing different restrictions on the messages that different modules publish and subscribe to. Some of these rules are probably too strict, and some too lax: we should feel free to change them as needed as we move forward and learn more.

When we clean up, we'd like to clear all the bindings that refer to a dispatch_t, so that they don't have dangling pointers to it.

This commit has the necessary logic to run the publish/subscribe system from the mainloop, and to initialize it on startup and tear it down later.

(The header won't compile without it.)

Also, add documentation, and fix a free-on-error bug.

We want the DISPATCH_ADD_PUB() macro to count as making a DECLARE_PUBLISH() invocation "used", so let's try a new approach that preserves that idea. The old one apparently did not work for some versions of osx clang.

(Our code to handle it was broken, too)

This is necessary to get the number of includes in main.c back under control. (In the future, we could just use the subsystem manager for this kind of stuff.)

Previously, I had used integers encoded as pointers. This introduced a flaw: NULL represented both the integer zero, and the absence of a setting. This in turn made the checks in cfg_msg_set_{type,chan}() not actually check for an altered value if the previous value had been set to zero. Also, I had previously kept a pointer to a dispatch_fypefns_t rather than making a copy of it. This meant that if the dispatch_typefns_t were changed between defining the typefns and creating the dispatcher, we'd get the modified version. Found while investigating coverage in pubsub_add_{pub,sub}_()

This helps diagnostics.

Also fix a grammar error in a comment.

Having the numbers in those messages makes some of the unit test unstable, by causing them to depend on the initialization order of the naming objects.

(main.c is a bit better, but shutdown.c is ugly)

coveralls · 2019-03-27T03:00:47Z

Pull Request Test Coverage Report for Build 4439

644 of 714 (90.2%) changed or added relevant lines in 14 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.3%) to 62.273%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/app/main/main.c	12	13	92.31%
src/lib/pubsub/pubsub_check.c	159	160	99.38%
src/lib/pubsub/pubsub_build.c	109	111	98.2%
src/lib/dispatch/dispatch_core.c	78	84	92.86%
src/app/main/subsysmgr.c	13	21	61.9%
src/app/main/shutdown.c	62	80	77.5%
src/core/mainloop/mainloop_pubsub.c	13	47	27.66%

Files with Coverage Reduction	New Missed Lines	%
src/core/or/relay.c	1	49.33%

Totals
Change from base Build 4435:	0.3%
Covered Lines:	46180
Relevant Lines:	74157

💛 - Coveralls

nmathewson added 30 commits March 25, 2019 16:35

Add a type to map names to short identifiers

dfd7a7f

We'll be using this for four kinds of identifier in dispatch.c

Add MESG as a new log domain.

0944500

Add a macro for the fairly common "eat next semicolon" syntax trick

56bda74

You use this when you're defining a macro to be used at file scope, and you want to require a semicolon afterwards.

Add a smartlist_grow() function to expand a smartlist

253fea8

Tests included.

Add a new inline function to check whether debug logging is on

a62ac17

We already do this in our log_debug() macro, but there are times when we'd like to avoid allocating or precomputing something that we are only going to log if debugging is on.

Add a naming system for IDs in dispatch.

f5683d9

Debug logs to record all messages sent and delivered.

24b945f

Add function to clear publish bindings.

a768152

When we clean up, we'd like to clear all the bindings that refer to a dispatch_t, so that they don't have dangling pointers to it.

Pubsub: macros for ease-of-use and typesafety.

24df14e

Code to manage publish/subscribe setup via subsystem interface.

bdeaf7d

This commit has the necessary logic to run the publish/subscribe system from the mainloop, and to initialize it on startup and tear it down later.

Add msgtypes.h include to pubsub_build.h

02e0a39

(The header won't compile without it.)

Connect the mainloop pubsub dispatcher on startup; free it on shutdown.

6d1abd3

pubsub: install libevent events separately from the_dispatcher.

b4f28b9

Also, add documentation, and fix a free-on-error bug.

Document several issues found by Taylor

3d6bf7b

Correct doxygen @file directives

22ad865

move pubsub_connector_t typedef to pubsub_connect.h

94feec5

Try a different approach to making publish function seem used.

beedadb

We want the DISPATCH_ADD_PUB() macro to count as making a DECLARE_PUBLISH() invocation "used", so let's try a new approach that preserves that idea. The old one apparently did not work for some versions of osx clang.

pubsub test: add a test to make sure typefns can't be changed.

9fb5115

Unit test for namemap_fmt_name()

f74301f

dispatch: Test behavior of formatting type with no set fmt function.

c40bcab

pubsub_check.c: Stop accepting NULL prefix, which we never send.

2e7f80d

(Our code to handle it was broken, too)

coverage: Exclude lines in pubsub_pub that can only be reached on bug

3552cd6

pubsub: add test for items_out in builder_finalize()

d976cda

practracker compliance: split lint_message into more logical parts

4bdff5e

practracker: split shutdown code out of main.c

ab6ddc7

This is necessary to get the number of includes in main.c back under control. (In the future, we could just use the subsystem manager for this kind of stuff.)

Add test for dispatch_send() fast path.

b11b4b7

Add a test for pubsub_items_clear_bindings()

8d70f21

nmathewson added 9 commits March 25, 2019 16:35

Log warning messages _before_ exiting because of earlier dcfg failure

a8ca464

This helps diagnostics.

Various documentation notes and tweaks for pubsub

28fd499

Rename DISPATCH_DEFINE_TYPE() to DISPATCH_REGISTER_TYPE()

3f0bfe1

Also fix a grammar error in a comment.

Rename one case of c_type to c_ptr_type.

ddb31dd

Remove message/subsystem numbers from log messages

203e913

Having the numbers in those messages makes some of the unit test unstable, by causing them to depend on the initialization order of the naming objects.

changes file for pubsub code

3767eff

Merge branch 'messaging_v3' into messaging_v3_merged

a47b61f

practracker updates from messaging_v3 merge

7b97320

(main.c is a bit better, but shutdown.c is ugly)

ghost merged commit 7b97320 into torproject:master Mar 27, 2019

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Messaging v3 merged: the big publish-subscribe branch of fun. #859

Messaging v3 merged: the big publish-subscribe branch of fun. #859

nmathewson commented Mar 27, 2019

coveralls commented Mar 27, 2019

Messaging v3 merged: the big publish-subscribe branch of fun. #859

Messaging v3 merged: the big publish-subscribe branch of fun. #859

Conversation

nmathewson commented Mar 27, 2019

coveralls commented Mar 27, 2019

Pull Request Test Coverage Report for Build 4439

💛 - Coveralls