5 C# Bugs That Pass Code Review (and the Interview Questions Hiding in Them)

The senior .NET interview questions that catch people out usually aren't trivia. They're a small piece of code and "would you approve this PR?" The code compiles, it looks tidy, and it has a bug that only shows up later: in production, under load, or on the one day the database is slow.

Below are five of those, written as the kind of pull request you'd actually see. For each one: what's wrong, the follow-up question the interviewer asks once you spot it (that's where most people stumble), and the review comment I'd leave. I ran the behavior behind each example on .NET 10 (SDK 10.0.401). The two facts I didn't run myself, the thread-pool crash and the Development-only scope check, come from Microsoft's docs, and I say so where they come up. I also keep a tiny project called InterviewTraps that pins the same five behaviors with 17 xUnit tests, so none of this rests on memory.

If you're preparing for interviews, the Ultimate .NET Interview Guide (200 Q&A) gives you 200 questions with answers to practice, plus an interactive code editor. For complete, tested .NET projects with full source code and PDF guides, there's Tech Skill Builder.

Tech Skill Builder is $5/month for a very limited time only (normally $10). Grab it now before it goes back to $10.

Bug 1: The try/catch that catches nothing

public sealed class AuditWriter
{
    public async void SaveAudit(string entry)
    {
        await Task.CompletedTask; // stands in for the real database call
        throw new InvalidOperationException("db down");
    }
}

// in the caller
try
{
    audit.SaveAudit(entry);
}
catch (Exception ex)
{
    logger.LogError(ex, "Audit failed"); // never runs
}

What's wrong: async void returns nothing the caller can observe, so the exception can't come back through the call. The catch block is dead code. In my check, with a recording SynchronizationContext installed, the caller's catch saw nothing and the exception landed on the context instead.

The follow-up: "So where does the exception go?" To the synchronization context that was current when the method started. If there isn't one (a console app, a worker, a thread-pool thread), it's rethrown on the thread pool, and an unhandled exception there takes the process down. That part is documented runtime behavior; I didn't run it, since it kills the test host.

My review comment: "Return Task and await it. async void is for event handlers only, and those get their own try/catch inside."

Bug 2: One query, three database trips

IEnumerable<Order> overdue = repository.GetAll()
    .Where(o => o.DueDate < today);

if (overdue.Any())
{
    logger.LogWarning("{Count} overdue orders", overdue.Count());
    foreach (var order in overdue)
    {
        // send a reminder
    }
}

What's wrong: overdue is a query, not a list. Any(), Count() and the foreach each run it again from the start. With an iterator-based repository that counts its calls, GetAll() ran 3 times for this one block.

The follow-up: "And if GetAll() returns an EF Core IQueryable?" Then it's three SQL queries, and the count and the loop can even disagree if a row changes in between. A second trap hides in the same code: the query reads today when it's enumerated, not when it's defined.

My review comment: "Materialize once with ToList() and use the list everywhere below."

Bug 3: The loop that processes the wrong batch

var tasks = new List<Task>();
for (var i = 0; i < batches.Count; i++)
{
    tasks.Add(Task.Run(() => Process(batches[i])));
}
await Task.WhenAll(tasks);

What's wrong: a for loop has one i for the whole loop, and every lambda captures that same variable. By the time a task runs, the loop has usually moved on. In my run, this threw ArgumentOutOfRangeException in 195 of 200 attempts, because most tasks read i after it had reached batches.Count. The other 5 "worked," which is how it survives a quick manual test.

The follow-up: "Does foreach have the same problem?" No. Since C# 5, foreach gets a fresh variable per iteration. This is why people who remember the old behavior give the wrong answer in both directions.

My review comment: "Copy i into a local inside the body, or use foreach over batches."

Bug 4: The retry loop that never stops

public struct RetryBudget
{
    public RetryBudget(int max) => Remaining = max;

    public int Remaining { get; private set; }

    public bool TryConsume()
    {
        if (Remaining == 0) return false;
        Remaining--;
        return true;
    }
}

public sealed class Downloader
{
    private readonly RetryBudget _budget = new(3);

    public async Task DownloadAsync(CancellationToken ct)
    {
        while (_budget.TryConsume())
        {
            // try the download, return on success
            // ...omitted in this post
        }
    }
}

What's wrong: _budget is a readonly field of a mutable struct. Calling a mutating method on it compiles, but the compiler runs it on a hidden defensive copy, so Remaining never goes down. A budget of 3 retries becomes unlimited retries. When I capped the loop at 10 for the test, it ran all 10. Remove readonly from the field and it stops at 3.

The follow-up: "Where else does this happen?" Anywhere you get a copy instead of the storage location: the List<T> indexer (list[0].TryConsume() mutates a temporary) and the foreach variable. Arrays are the exception, array[0] is the real element.

My review comment: "Make it a readonly struct that returns a new value, or make it a class. A mutable struct in a readonly field is a bug waiting for a slow day."

Bug 5: The singleton that keeps yesterday's DbContext

builder.Services.AddDbContext<ShopDb>(o => o.UseSqlServer(connectionString)); // scoped
builder.Services.AddSingleton<PriceCache>();

public sealed class PriceCache(ShopDb db)
{
    // reads prices through db...
}

What's wrong: a singleton captured a scoped service. The first ShopDb it gets is kept for the life of the app and shared by every request on every thread, and a DbContext isn't thread-safe. This is the classic captive dependency. With a plain BuildServiceProvider(), the container allows it without a word. Turn on ValidateScopes and ValidateOnBuild and startup fails with a message like "Cannot consume scoped service 'ShopDb' from singleton 'PriceCache'."

The follow-up: "Then why did it work on my machine?" Because ASP.NET Core's default host turns on scope validation only in the Development environment, according to Microsoft's docs. Staging and Production, or a test that builds its own provider, get no check unless you opt in.

My review comment: "Make PriceCache scoped, or inject IServiceScopeFactory and create a scope per refresh:"

public sealed class PriceCache(IServiceScopeFactory scopes)
{
    public async Task RefreshAsync(CancellationToken ct)
    {
        await using var scope = scopes.CreateAsyncScope();
        var db = scope.ServiceProvider.GetRequiredService<ShopDb>();
        // load prices and swap the cached snapshot
        // ...omitted in this post
    }
}

Prove it instead of arguing about it

The fastest way to settle any of these in a real code review is a ten-line test. Here's the shape I use for the captive dependency:

[Fact]
public void Singleton_captures_the_first_scoped_instance()
{
    // RequestContext: a plain scoped class. ReportGenerator: a singleton that takes one.
    using var provider = new ServiceCollection()
        .AddScoped<RequestContext>()
        .AddSingleton<ReportGenerator>()
        .BuildServiceProvider();

    using var scopeA = provider.CreateScope();
    using var scopeB = provider.CreateScope();
    // resolve RequestContext and ReportGenerator in both scopes, then assert
    // the two scopes differ but both generators share one context
    // (remaining asserts trimmed for this post)
}

The InterviewTraps project has one test file per bug like this, 17 tests in total, and all of them pass on .NET 10.

Quick self-check before your next interview

• [ ] Can I explain where an async void exception goes, with and without a synchronization context?
• [ ] Do I know how many times a LINQ query runs, and when it reads captured variables?
• [ ] Can I say why for and foreach capture differently, and since which C# version?
• [ ] Can I name three places a struct gets silently copied?
• [ ] Do I know which environments validate DI scopes by default?

If any box stays empty, that's the question to practice first.

Get the full prep kit

These five are a sample of the kind of question that separates a senior answer from a memorized one. The Ultimate .NET Interview Guide (200 Q&A) gives you 200 .NET interview questions with answers in one place, plus an interactive code editor, for a one-time $10. If one of the bugs above got past you, go through the rest before an interviewer does it for you.

And if you learn best by reading and running real code, Tech Skill Builder gives you complete, working, tested .NET projects with full source code and step-by-step PDF guides, with new ones added regularly. Tech Skill Builder is $5/month for a very limited time only (normally $10). Grab it now before it goes back to $10. 👉 Join Tech Skill Builder

Which of the five would you have approved? Be honest. My bet is the struct one, because the defensive copy is invisible in a diff.