Skip to content

LineRewriter: _isSetCursorPositionSupported cache depends on instance-specific LineToClear value #5780

@mdh1418

Description

@mdh1418

Just came across some slightly confusing LineRewriter behavior when looking into PR feedback #5771 (comment)

I don't think it's high priority, but would be nice to clarify at some point.

Summary

LineRewriter.IsRewriteConsoleLineSupported probes console capability by calling SetCursorPosition(0, LineToClear), but caches the result in a static field. Since LineToClear is a mutable, instance-specific property, the cached result reflects whether a specific row is valid — not whether the console supports cursor repositioning in general.

Current behavior

// LineRewriter.cs
public int LineToClear { get; set; }                    // mutable, instance-specific
private static bool? _isSetCursorPositionSupported;      // cached globally

public bool IsRewriteConsoleLineSupported
{
    get => _isSetCursorPositionSupported ?? EnsureInitialized();

    bool EnsureInitialized()
    {
        try
        {
            Console.SetCursorPosition(0, LineToClear);   // probes with instance value
            _isSetCursorPositionSupported = true;         // caches globally
        }
        catch
        {
            _isSetCursorPositionSupported = false;        // caches globally
        }
    }
}

If LineToClear is -1 (which happens when CursorTop is 0 in no-TTY environments), SetCursorPosition throws, and the static cache permanently records false — even if the console genuinely supports cursor repositioning and a subsequent LineRewriter instance has a valid LineToClear.

Why this doesn't cause issues today

dotnet-trace runs one command per process invocation. collect and collect-linux can't run in the same process. Each invocation creates a single LineRewriter instance with one LineToClear value. The static cache is effectively instance-scoped by accident.

Additionally, PR #5771 added a lineToClear < 0 guard in collect-linux's ProgressWriter that prevents probing with an invalid value.

Design questions

  1. Should _isSetCursorPositionSupported be instance-scoped? The cached result depends on LineToClear, which is instance state. A static cache is only correct if all instances share the same LineToClear — which happens to be true today but isn't guaranteed by the API.

  2. Should the probe test console capability independently of LineToClear? For example, probing with the current cursor position (SetCursorPosition(left, top)) would test "can this console reposition at all?" without conflating it with "is this specific row valid?" But this changes the semantics — collect relies on the probe to validate its specific LineToClear value.

  3. Should LineToClear be immutable? It's set once after construction in both callers (collect and collect-linux), with one exception: collect does LineToClear-- as a one-time adjustment when the cursor is at the buffer bottom. Making it a constructor parameter would make the API harder to misuse.

  4. Should RewriteConsoleLine() validate LineToClear internally? Currently callers must check IsRewriteConsoleLineSupported before calling RewriteConsoleLine(). If they don't, and LineToClear is invalid, it crashes. A self-guarding RewriteConsoleLine would be safer.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority:3Work that is nice to have

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions