Re: [chromium-dev] Re: ScopedTaskEnviroment, MOCK_TIME and WaitableEvent

[dan...@chromium.org]

+scheduler-dev 

On Fri, Jul 19, 2019 at 9:49 PM Yuchen Liu <yuc...@chromium.org> wrote:

Any idea on this?

On Friday, July 19, 2019 at 10:14:30 AM UTC-7, Yuchen Liu wrote:

Hi, we have a unit test similar to this one:

TEST_F(ScopedTaskEnvironmentTest, WaitableEvent) {

  ScopedTaskEnvironment scoped_task_environment(

      ScopedTaskEnvironment::TimeSource::MOCK_TIME);

  WaitableEvent event(WaitableEvent::ResetPolicy::AUTOMATIC,

                                    WaitableEvent::InitialState::NOT_SIGNALED);

  PostDelayedTask(

      FROM_HERE,

      base::BindOnce([](WaitableEvent* event) { event->Signal(); }, &event),

      TimeDelta::FromMilliseconds(100));

  event.Wait();

  scoped_task_environment.FastForwardUntilNoTasksRemain();

}

Previously we use MainThreadType::MOCK_TIME, according to the code comment, MainThreadType::MOCK_TIME is deprecated by TimeSource::MOCK_TIME. However the test never ends with latest code on linux. It works before we upgrade the code to TOT. "event.Wait()" never returns. I think the task posted never executes. Do I setup the test in a wrong way?

Thanks!

Yuchen

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/58a6a9b7-d54a-4740-8811-29e34fbb6d34%40chromium.org.

[Carlos Caballero Grolimund]

Hola!

I am not sure what you are trying to do here. Wait will never return, because the task will run when you call FastForwardUntilNoTasksRemain. Maybe if you pointed me to the real test I could have a look.

Cheers

Carlos

On Mon, Jul 22, 2019 at 4:49 PM <dan...@chromium.org> wrote:

+scheduler-dev 

On Fri, Jul 19, 2019 at 9:49 PM Yuchen Liu <yuc...@chromium.org> wrote:

Any idea on this?

On Friday, July 19, 2019 at 10:14:30 AM UTC-7, Yuchen Liu wrote:

Hi, we have a unit test similar to this one:

TEST_F(ScopedTaskEnvironmentTest, WaitableEvent) {

  ScopedTaskEnvironment scoped_task_environment(

      ScopedTaskEnvironment::TimeSource::MOCK_TIME);

  WaitableEvent event(WaitableEvent::ResetPolicy::AUTOMATIC,

                                    WaitableEvent::InitialState::NOT_SIGNALED);

  PostDelayedTask(

      FROM_HERE,

      base::BindOnce([](WaitableEvent* event) { event->Signal(); }, &event),

      TimeDelta::FromMilliseconds(100));

  event.Wait();

 This will never return, as your task will not get a chance to run as this happens in the next line.

  scoped_task_environment.FastForwardUntilNoTasksRemain();

If you reorder these two lines the test would run.

}

Previously we use MainThreadType::MOCK_TIME, according to the code comment, MainThreadType::MOCK_TIME is deprecated by TimeSource::MOCK_TIME. However the test never ends with latest code on linux. It works before we upgrade the code to TOT. "event.Wait()" never returns. I think the task posted never executes. Do I setup the test in a wrong way?

Thanks!

Yuchen

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/58a6a9b7-d54a-4740-8811-29e34fbb6d34%40chromium.org.

--

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/CAHtyhaQgpjJLe0jKxckojxgb6AEY9brsdC%2B6qqbLFpM5Yurm-w%40mail.gmail.com.

[Yuchen Liu]

Hi Carlos, thanks for the help!

This is our internal test. The code snippet in this email simplifies the original code with the same logic. I think we expect task posted by PostDelayedTask to run on a background thread so that the main thread won't be blocked. The same test works before. But we're using MainThreadType::MOCK_TIME by that time. So I'm wondering if my setup of the test is correct or not.

[Carlos Caballero Grolimund]

Ok, got it. Sorry I thought PostDelayedTask was your own method. It is the one in post_task.h so it posts to the ThreadPool by default.

Gab recently added support for MOCK_TIME in the thread pool, so the clock that delayed tasks use to run will only advance when reaching idle during a FastForward* call or if a RunLoop::Run call goes idle on the main thread.

Since you call non of those methods before you call event.Wait(); time will not advance and thus the event will not trigger.

Your tests should work if you used ScopedTaskEnvironment::TimeSource::SYSTEM_TIME instead. Bu I am not quite sure what you are actually testing here. Why do you need to post a delayed task?

[Carlos Caballero Grolimund]

Gab, I wonder if we should clarify a bit more the comment for ScopedTaskEnvironment::ScopedTaskEnvironment::ASYNC, make it clearer that with a MOCK_TIME ASYNC actually behaves like QUEUED for delayed tasks.

[Gabriel Charette]

Carlos is correct, MOCK_TIME now applies to all threads, so delayed tasks will never run unless you invoke a ScopedTaskEnvironment::FastForward*() method or RunLoop::Run(). You can write the same test correctly this way:

TEST_F(ScopedTaskEnvironmentTest, WaitableEvent) {

  ScopedTaskEnvironment scoped_task_environment(

      ScopedTaskEnvironment::TimeSource::MOCK_TIME);

  RunLoop run_looop;

  PostDelayedTask(

      FROM_HERE,

      run_loop.QuitClosure(),

      TimeDelta::FromMilliseconds(100));

  run_loop.Run();

  // Note that this will run all tasks, delayed or not, on the main thread

  // and in the thread pool until both are idle.

  scoped_task_environment.FastForwardUntilNoTasksRemain();

}

On Tue, Jul 23, 2019 at 1:01 PM Carlos Caballero Grolimund <carl...@google.com> wrote:

Gab, I wonder if we should clarify a bit more the comment for ScopedTaskEnvironment::ScopedTaskEnvironment::ASYNC, make it clearer that with a MOCK_TIME ASYNC actually behaves like QUEUED for delayed tasks.

Hmmm, that'd be true but was that the source of confusion? IMO the docs for MOCK_TIME applying to all threads makes delayed tasks implicitly "queued"?

[Carlos Caballero Grolimund]

On Wed, Jul 24, 2019 at 1:06 PM Gabriel Charette <g...@chromium.org> wrote:

Carlos is correct, MOCK_TIME now applies to all threads, so delayed tasks will never run unless you invoke a ScopedTaskEnvironment::FastForward*() method or RunLoop::Run(). You can write the same test correctly this way:

TEST_F(ScopedTaskEnvironmentTest, WaitableEvent) {

  ScopedTaskEnvironment scoped_task_environment(

      ScopedTaskEnvironment::TimeSource::MOCK_TIME);

  RunLoop run_looop;

  PostDelayedTask(

      FROM_HERE,

      run_loop.QuitClosure(),

      TimeDelta::FromMilliseconds(100));

  run_loop.Run();

  // Note that this will run all tasks, delayed or not, on the main thread

  // and in the thread pool until both are idle.

  scoped_task_environment.FastForwardUntilNoTasksRemain();

}

On Tue, Jul 23, 2019 at 1:01 PM Carlos Caballero Grolimund <carl...@google.com> wrote:

Gab, I wonder if we should clarify a bit more the comment for ScopedTaskEnvironment::ScopedTaskEnvironment::ASYNC, make it clearer that with a MOCK_TIME ASYNC actually behaves like QUEUED for delayed tasks.

Hmmm, that'd be true but was that the source of confusion? IMO the docs for MOCK_TIME applying to all threads makes delayed tasks implicitly "queued"?

I don't know if it was the source. Just something I noticed while reading them with this in mind. I would mention it the description of  ASYNC that this might be affected by MOCK_TIME. Just to be crystal clear.

[Gabriel Charette]

On Wed, Jul 24, 2019 at 11:16 AM Carlos Caballero Grolimund <carl...@google.com> wrote:

On Wed, Jul 24, 2019 at 1:06 PM Gabriel Charette <g...@chromium.org> wrote:

Carlos is correct, MOCK_TIME now applies to all threads, so delayed tasks will never run unless you invoke a ScopedTaskEnvironment::FastForward*() method or RunLoop::Run(). You can write the same test correctly this way:

TEST_F(ScopedTaskEnvironmentTest, WaitableEvent) {

  ScopedTaskEnvironment scoped_task_environment(

      ScopedTaskEnvironment::TimeSource::MOCK_TIME);

  RunLoop run_looop;

  PostDelayedTask(

      FROM_HERE,

      run_loop.QuitClosure(),

      TimeDelta::FromMilliseconds(100));

  run_loop.Run();

  // Note that this will run all tasks, delayed or not, on the main thread

  // and in the thread pool until both are idle.

  scoped_task_environment.FastForwardUntilNoTasksRemain();

}

On Tue, Jul 23, 2019 at 1:01 PM Carlos Caballero Grolimund <carl...@google.com> wrote:

Gab, I wonder if we should clarify a bit more the comment for ScopedTaskEnvironment::ScopedTaskEnvironment::ASYNC, make it clearer that with a MOCK_TIME ASYNC actually behaves like QUEUED for delayed tasks.

Hmmm, that'd be true but was that the source of confusion? IMO the docs for MOCK_TIME applying to all threads makes delayed tasks implicitly "queued"?

I don't know if it was the source. Just something I noticed while reading them with this in mind. I would mention it the description of  ASYNC that this might be affected by MOCK_TIME. Just to be crystal clear.

SGTM https://chromium-review.googlesource.com/c/chromium/src/+/1716914