Aspnetcore.docs: Safety of incrementing executionCount

[thanuja89]

I might be missing something here. Is incrementing the executionCount safe like this, without syncronization or using interlocked operations?

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: c3460b0d-9c60-3a06-ac63-18cf990e53c3
• Version Independent ID: b2dece1f-15b8-03af-6d52-7cc070432b4b
• Content: Background tasks with hosted services in ASP.NET Core
• Content Source: aspnetcore/fundamentals/host/hosted-services.md
• Product: aspnet-core
• Technology: aspnetcore-fundamentals
• GitHub Login: @guardrex
• Microsoft Alias: riande

[guardrex]

Hello @thanuja89 ... It's fine in these cases. executionCount variables in these services are local to each service. Nothing external to each service can access them.

[thanuja89]

Hi @guardrex..

Correct me if I'm wrong here. The TimedHostedService uses System.Threading.Timer which queues callbacks to the treadpool. According to my understanding, it is not guaranteed that all the previous callbacks have finished execution when they run. So I'm assuming there may be two callbacks executing at the same time. If that is the case, the executionCount could be easily wrong.

The other concern is these callbacks are more likely to be run in different threads, so the executionCount could be cached in the CPU. If the code is run on a machine with more than one processor, each processor could cache the executionCount and increment that value (since non-volatile writes are used).

What are your thoughts on this @guardrex?

[guardrex]

Ah! I see what you mean now.

I think your first concern is very real. In this silly example that we have, it should be fine. We probably need to at least add a remark on it so that devs are warned.

The second point needs to be addressed by a REAL developer (not a hack like me :smile: lol) ... @huan086, do you have a moment to address that one, and what do you think about @thanuja89's first point, too?

For reference, here's the service :point_right: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-3.1&tabs=visual-studio#timed-background-tasks

Let's keep in mind something as we sort this out: We might be able to stick with the silly example and cross-link to MS content that shows how to avoid these issues. If not, we'll have to do something else with the example so that we're not guiding readers with a pattern that's going to :boom: on them in real apps.

[huan086]

Yup a reminder that System.Threading.Timer does not wait for previous execution to finish, and may run in different threads would be great. Especially to people who started from WinForms (me), which has timer that "just works safely".

To fix the code, I think this would be sufficient:

Replace

executionCount++;
_logger.LogInformation(
            "Timed Hosted Service is working. Count: {Count}", executionCount);

with

int newCount = Interlocked.Increment(ref executionCount);
_logger.LogInformation(
            "Timed Hosted Service is working. Count: {Count}", newCount);

[guardrex]

Thanks @thanuja89 and @huan086 ... I'll get the topic+sample updated.