Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential bug with race condition #484

Open
Ulimo opened this issue Jun 24, 2023 · 4 comments
Open

Potential bug with race condition #484

Ulimo opened this issue Jun 24, 2023 · 4 comments

Comments

@Ulimo
Copy link

Ulimo commented Jun 24, 2023

Hi, this might be a user error, but I got the examples working and got correct bug reports.

But the following code:

[Fact]
public async Task Test()
{
    long counter = 17;

    var task1 = Task.Run(() =>
    {
        var val = counter;
        counter -= val;
    });
    var task2 = Task.Run(() =>
    {
        var val = counter;
        counter -= val;
    });

    await Task.WhenAll(task1, task2);

    Assert.Equal(0, counter);
}

[Fact]
[Microsoft.Coyote.SystematicTesting.Test]
public void CoyoteTest()
{
    var configuration = Configuration.Create().WithTestingIterations(10);
    var engine = TestingEngine.Create(configuration, Test);
    engine.Run();
}

Does not produce any bugs. in some cases this can return -17 instead of 0, but I cant get coyote to produce that result.

Changing to this:

[Fact]
public async Task Test()
{
    long counter = 17;

    var task1 = Task.Run(async () =>
    {
        var val = counter;
        await Task.Delay(1);
        counter -= val;
    });
    var task2 = Task.Run(async () =>
    {
        var val = counter;
        await Task.Delay(1);
        counter -= val;
    });

    await Task.WhenAll(task1, task2);

    Assert.Equal(0, counter);
}

produces the bug report correctly, is this expected behaviour or should the first example also produce a bug?

@akashlal
Copy link
Collaborator

This is expected behavior. By default, Coyote does not interrupt tasks at memory accesses. It only interrupts them at certain Task APIs (such as the await that you inserted in the second program). You can try the flag --rewrite-memory-locations. It is experimental, but it would add scheduling points at memory accesses.

@Ulimo
Copy link
Author

Ulimo commented Jun 24, 2023

Thank you for the quick response, rewrite memory locations was already default set to true, but then I know!

@Ulimo
Copy link
Author

Ulimo commented Jun 24, 2023

I changed my code to this and it seems to produce the correct results

[Fact]
public async Task Test()
{
    long counter = 17;

    var task1 = Task.Run(() =>
    {
        var val = counter;
        SchedulingPoint.Interleave();
        counter -= val;
    });
    var task2 = Task.Run(() =>
    {
        var val = counter;
        SchedulingPoint.Interleave();
        counter -= val;
    });

    await Task.WhenAll(task1, task2);

    Assert.Equal(0, counter);
}

So for now I guess I just have to add interleave to places where I want it to check.

@pdeligia
Copy link
Member

pdeligia commented Jun 26, 2023

Thanks @Ulimo and @akashlal, you also need to enable: Configuration.WithMemoryAccessRaceCheckingEnabled (or if you run from command line: --enable-memory-access-races). The --rewrite-memory-locations flag is for enabling/disabling the rewriting itself (which is indeed on by default), but you also have to enable the feature during testing (off by default to avoid exploding the schedule-space).

@Ulimo can you try this and let us know? (But the automated memory-access interleaving feature is indeed experimental and might not work for every single scenario as Akash said.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants