Fix race in parallel adjacent_find

[Casey Carter (CaseyCarter)]

Check cancellation status before getting a chunk of work to process, instead of after. If we check after and a result was found in a later block than the one we were assigned, we won't notice a better result in our block.

I think this is the cause of the extremely infrequent test failures we're seeing, e.g. https://dev.azure.com/vclibs/STL/_build/results?buildId=2983&view=ms.vss-test-web.build-test-results-tab&runId=1006240&resultId=115817&paneView=debug.

[Billy O'Neal (BillyONeal)]

Also checked the other find algorithms and they already did this.

[Stephan T. Lavavej (StephanTLavavej)]

Great catch! This looks correct to me, but Miya Natsuhara (@mnatsuhara) is more qualified to provide final signoff.

After this change, I observe that we now have 8 occurrences of the exact same pattern:

STL/stl/inc/execution

Lines 1291 to 1298 in 11150ca

	if (_Results._Complete()) {
	return _Cancellation_status::_Canceled;
	}
	const auto _Key = _Team._Get_next_key();
	if (!_Key) {
	return _Cancellation_status::_Canceled;
	}

(_Static_partitioned_mismatch2::_Process_chunk() is very slightly different, testing _Results._Storage._Complete(), but it could be made completely uniform by adding a helper function to _Static_partitioned_mismatch_results.)

Can/should we centralize this pattern? It looks like intrusively modifying _Static_partition_team::_Get_next_key() to take _Results and inspect _Results._Complete() would disrupt other _Get_next_key() callsites and mix in something that the _Static_partition_team shouldn't be concerned with, but could we add a non-member function that takes both _Results and _Team? The callers will still need if (!_Key) { return _Cancellation_status::_Canceled; } but this would give us an opportunity to add a centralized comment about the surprisingly subtle control flow here (otherwise only captured in your PR description) and would make it structurally easier to do the right thing in the future (for new algorithms or maintenance of existing algorithms).

[Casey Carter (CaseyCarter)]

Also checked the other find algorithms and they already did this.

I spent about an hour tracing through code looking for the race, maybe two minutes figuring out how to fix it, roughly a minute discovering that all of the other _find-like algorithms already had the same solution, and fifteen minutes raging that I could have saved the hour by simply looking at the other algorithms first ;)

After this change, I observe that we now have 8 occurrences of the exact same pattern:
Can/should we centralize this pattern? ... could we add a non-member function that takes both _Results and _Team?

Latest changes include a non-member function template _Get_next_if_not_canceled that accepts either a _Cancellation_token or any argument x for which x._Complete() returns bool for its first argument, and a _Static_partition_team for its second. This let me consolidate 11 occurrences of "Are we finished? Ok, then get a chunk of work." with a nice comment.

I'll reset reviews since this is a much more substantial change than the initial bugfix.

[Casey Carter (CaseyCarter)]

Can/should we centralize this pattern?

This is once again unresolved: I reverted my attempt after discovering detrimental codegen.

[Stephan T. Lavavej (StephanTLavavej)]

Thanks for double-checking the codegen!

[Casey Carter (CaseyCarter)]

Thanks race condition! for fixing this