Improve inline testing example

[adsteel]

Problem

Previous example depended on the manager having been started via Run
or RunWithContext. This kicks off goroutines that result in data races
and flaky builds.

Solution

Make threadsafe manager setup method public. Update readme to explain how
to avoid requiring a call to mgr.Run* and the resulting data races.

[mperham]

This is not safe because syntheticPush is still not thread-safe. You're reassigning a new Pool to mgr for each call.

[adsteel]

@mperham pool is only being reassigned if it is not already assigned. I may be missing something, but is that not thread safe?

[mperham]

Ah, right. That's far less bad but still unsafe. That pattern is called Check and Set. Another thread can execute between the Check and the Set.

[mperham]

I would move that pool management inside mgr so you can wrap it in a Mutex.

[adsteel]

@mperham ah, that makes sense. I switched tack to reuse existing, thread safe logic that sets manager defaults, by exposing mgr.setUpWorkerProcess as public. Readme, commit and PR message are updated, and I just ran my test suite against this change and things look good so far.

[mperham]

If you’re adding public APIs, we’ll need a much stricter review.

[mperham]

I don't like this direction. You're asking the user to embed a lot of mgr setup, and therefore making it impossible to refactor that logic in future versions.

What about providing a mgr.InlineDispatch(job) method instead?

[adsteel]

You're asking the user to embed a lot of mgr setup, and therefore making it impossible to refactor that logic in future versions.

Agreed, it's not ideal for maintenance.

What about providing a mgr.InlineDispatch(job) method instead?

That does feel more sustainable. It seems like it would be used exclusively for tests, yeah?

I just pushed up an implementation like that. What do you think of this direction?

[adsteel]

This allows us to make the recently introduced mgr.Dispatch and mgr.IsRegistered private, for a net gain of one public method (that now needs tests).