On Mon Aug 22 19:06:46 2022 +0000, Piotr Caban wrote:
I've pushed modified version of your patches, here's a short description of changes:
- remove unneeded changes from this series (e.g. ExceptionPtr class is
not used yet)
- don't call Scheduler_Attach when adding new task - this part is a
little tricky, in your implementation we might leak scheduler reference if ScheduleTask fails, changed code depends on scheduler task running under the same scheduler that creates it (which is not enforced yet but should be true in most cases) Does it look good for you?
Hello,
This will fail the test "test that scheduled chores are executed even if _RunAndWait is not called" if you create and attach a new Scheduler to the main thread, since the chore threads currently don't inherit the Scheduler.
A quick test shows that the chore threads do inherit the attached Scheduler, so once we implement that, this implementation should work without problem.
(I will need to test if this inheriting of the Scheduler is a property of StructuredTaskCollection or of ThreadScheduler. I suspect the latter but I could be wrong)
I see that you removed my attempt at being thread safe in `schedule_chore`. I'm not sure how native msvcr handles concurrency but the function is probably not intended to be thread-safe anyway so no complaints there.
I originally put the `operator_new` call above the `this->context` assignment so that if allocation fails we wouldn't leave `this->context` set to non-NULL. But I also never tested this (I don't even know how to trigger this case in native msvcr) so I guess this is fine too.