Nested task spawning broken

[rob-cohen]

When I use nested tasks with async::spawn() and async::get(), the synchronization doesn't work properly. This happens pretty consistently, and I am forced to never spawn tasks from tasks, which can be very limiting.

In the example below, some of the get()s will return before the work has completed, causing bugs.

Aside from this, the library is very convenient and works well.

Thanks!
Rob

void Function1()
{
	async::task<void> tasks[100];
	for (int t=0; t<100; t++)
	{
		tasks[t] = async::spawn([=]
		{
			Function2();
		});
	}

	for (int t=0; t<100; t++)
		tasks[t].get();
}

void Function2()
{
	async::task<void> tasks[100];
	for (int t=0; t<100; t++)
	{
		tasks[t] = async::spawn([=]
		{
			DoWork();
		});
	}

	for (int t=0; t<100; t++)
		tasks[t].get();
}

[rob-cohen]

Commenting out the task stealing in threadpool_scheduler.cpp avoids the bug...

/*
			// Try to steal a task
			if (task_run_handle t = steal_task(impl, thread_id)) {
				t.run();
				break;
			}
*/

Perhaps the task stealing breaks assumptions related to task synchronization?

As you probably know, task stealing can be very desirable to maintain good CPU utilization and, in some cases, to avoid starvation deadlocks on shared resources.

Thanks!
Rob

[Amanieu]

Can you provide a full .cpp file that you use for testing this? I will look into it.

[rob-cohen]

This explains why the bug appears only for nested get()s. That's when task stealing comes into play. Again, the symptom is that get()s sometimes return before the task has completed.

[rob-cohen]

Can you provide a full .cpp file that you use for testing this? I will look into it.

It's part of a pretty big project. Perhaps I can put a simple repro test together that I can share.

[rob-cohen]

I'll go ahead and do that now. Thanks for your help!

[rob-cohen]

This is 100% reliable with NESTED_ASYNC 0, but often crashes with NESTED_ASYNC 1
It becomes 100% reliable with NESTED_ASYNC 1 when I disable task stealing in threadpool_scheduler.cpp (as shown in my second post.)

#include "async++.h"
#define NESTED_ASYNC 1
const int kTestCount = 10;
int main(int argc, char **argv)
{
	for (int x=0; x<100; x++)
	{
#if NESTED_ASYNC
		async::task<void> itasks[kTestCount];
#endif
		for (int i=0; i<kTestCount; i++)
		{
#if NESTED_ASYNC
			itasks[i] = async::spawn(
			[=]
			{
#endif
				async::task<void> jtasks[kTestCount];
				std::mutex mut;

				for (int j=0; j<kTestCount; j++)
				{
					jtasks[j] = async::spawn(
					[=, &mut]
					{
						int repeat = rand()%50;
						for (int r=0; r<repeat; r++)
						{
							mut.lock();
							mut.unlock();
						}
					});
				}

				for (int j=0; j<kTestCount; j++)
					jtasks[j].get();
#if NESTED_ASYNC
			});
#endif
		}

#if NESTED_ASYNC
		for (int i=0; i<kTestCount; i++)
			itasks[i].get();
#endif
	}

	return 0;
}

[rob-cohen]

Did this clip reproduce the problem for you? I am using MSVC2017/Windows.

[Amanieu]

Yes, I managed to reproduce the issue on Linux. I've been a bit busy lately, but I am looking into it.

[rob-cohen]

Yes, I managed to reproduce the issue on Linux. I've been a bit busy lately, but I am looking into it.

Great! Thanks for the update!

[rob-cohen]

I just tried the fix. It's more reliable now, but my code above still crashes in release mode on MSVC2017 if I increase kTestCount to 100. Thanks!

[rob-cohen]

Actually, a Rebuild All seemed to fix it! I'll do more testing...

[rob-cohen]

Unfortunately, it looks like it still isn't 100% reliable. I get a rare null reference exception in task_wait_event.h on notify_one():

	// Signal an event and wake up a sleeping thread
	void signal(int event)
	{
		std::unique_lock<std::mutex> lock(mutex());
		event_mask |= event;
		lock.unlock();
		cond().notify_one();  // null reference exception here
	}

[Amanieu]

I just pushed a fix, can you try again?

[rob-cohen]

Thanks! On it.