No GC Collect Call

ID

csharp.no_gc_collect_call

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

code_smell, garbage-collection, memory, performance

Description

Reports calls to GC.Collect. An explicit collection overrides the runtime’s own decision about when collecting pays off, blocks every managed thread while it runs, and normally costs more throughput than the memory it reclaims.

Rationale

The .NET collector is generational and self-tuning: it measures allocation rate and survival rate and collects when the cost is lowest. GC.Collect short-circuits that model. A forced full collection walks all generations, and every object that is still reachable gets promoted to an older generation — so the objects that survive the forced pass become more expensive to collect later, not cheaper. Meanwhile all managed threads are suspended for the duration.

The memory the call appears to free would have been reclaimed by the next natural collection anyway. A GC.Collect in production code is almost always a workaround for a leak that is really a lingering strong reference, an undisposed handle, or a static cache that is never trimmed — and calling the collector hides the symptom without fixing any of those.

Other GC members are not reported: GC.SuppressFinalize is mandated by the dispose pattern, GC.KeepAlive is the documented way to keep an object alive across a native call, and GC.AddMemoryPressure is the supported way to account for unmanaged allocations.

Test code is not reported either. Forcing a deterministic collection before measuring is a standard technique for a leak-regression test — often the only way such a test can observe that memory a previous iteration allocated was actually reclaimed — and is not the production performance smell this rule is otherwise about.

using System;

public class Cache
{
    private byte[] buffer = new byte[1024];

    public void Trim()
    {
        buffer = null;
        GC.Collect();                            // FLAW — forces a full blocking collection
    }

    public void Release(IntPtr handle)
    {
        GC.SuppressFinalize(this);               // OK — required by the dispose pattern
        GC.KeepAlive(handle);                    // OK — keeps the handle alive across a native call
        GC.AddMemoryPressure(4096);              // OK — accounts for an unmanaged allocation
    }
}

Remediation

Delete the call and let the runtime schedule collections. If the code is trying to release memory, find what is still holding a reference: dispose IDisposable objects (preferably with using), unsubscribe from events, and bound the size of long-lived caches. If the code is trying to release an unmanaged resource deterministically, implement IDisposable and dispose it rather than waiting for a finalizer.

The rare legitimate use — a benchmark harness or a memory-profiling test that needs a clean baseline — belongs in test code, not in the shipped path.

References