threading_and_tasks_faq.md g_condition example

[Nigel Tao]

https://chromium.googlesource.com/chromium/src/+/main/docs/threading_and_tasks_faq.md#how-to-test-code-that-posts-tasks

gives this example:

```
int g_condition = false;

base::RunLoop run_loop;
base::ThreadPool::PostTask(FROM_HERE, {}, base::BindOnce(
[] (base::OnceClosure closure) {
g_condition = true;
std::move(quit_closure).Run();
}, run_loop.QuitClosure()));

// Runs tasks until the quit closure is invoked.
run_loop.Run();

EXPECT_TRUE(g_condition);
```

Is it legit that the g_condition is just an int and there are no
mutexes or atomics involved, even if the lambda could run on a
different thread?

I note that https://chromium.googlesource.com/chromium/src/+/main/docs/threading_and_tasks.md#Memory-ordering-guarantees-for-posted-Tasks
was recently edited
(https://chromium-review.googlesource.com/c/chromium/src/+/4261928 and
https://chromium-review.googlesource.com/c/chromium/src/+/4283738) to
say

> This task system guarantees that all the memory effects of sequential execution before posting a task are visible to the task when it starts running

But that only talks about "before posting a task" and "visible to the
task": that changes during task-set-up are seen by the task. I'm
asking if changes during task-running are seen after the run_loop
quits. And IIUC the `std::move(quit_closure).Run()` call (1) isn't
actually posting another task but (2) is still running in a separate
thread.

I'm pretty sure the answer is "it's the same; it's legitimate" but
memory ordering is notoriously subtle and I am not an expert.

Like, this is just wild hypothesizing, but if 'synchronization'
happens 'just before a task starts' but not also 'just after a task
ends' then "pre-PostTask set-up is guaranteed visible in the task"
could be true but whether the "g_condition = true" write is
'synchronized' (and visible to the run_loop thread) might not be true
until the next ThreadPool task starts, and there might not be a next
task.

Should the "Memory ordering guarantees " section be word-smithed yet-again?

That is, of course, easy for me to say since I don't actually have a
proposal for what to word-smith the text *to*, other than to drop the
"for posted Tasks" from the section header.

[Wez]

RunLoop::QuitClosure().Run() necessarily coordinates with the RunLoop, to quit it - in practice it does so by posting a task to the RunLoop's message-loop, to quit it.

--
You received this message because you are subscribed to the Google Groups "scheduler-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scheduler-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/scheduler-dev/CAEdON6Ytk%3DtoLpT5gdK96LN4O-K2vo7qqnD%2Bb6Es%2BU%2BO8kgCww%40mail.gmail.com.

[Gabriel Charette]

On Fri, Mar 17, 2023 at 7:38 AM Wez <w...@chromium.org> wrote:

RunLoop::QuitClosure().Run() necessarily coordinates with the RunLoop, to quit it - in practice it does so by posting a task to the RunLoop's message-loop, to quit it.

Exactly, see RunLoop::QuitClosure()'s implementation. I agree that this would be a nice explicit addition to the documentation however, happy to review.