Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 259 additions & 0 deletions docs/rules/GD0002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
# GD0002: InputEvent not disposed

| Property | Value |
|----------|-------|
| Rule ID | GD0002 |
| Category | Memory |
| Severity | Warning |

## Cause

An `InputEvent` parameter in `_Input`, `_UnhandledInput`, or `_GuiInput` override methods is not properly disposed after extracting needed data.

## Rule description

Godot's C# implementation has a known memory leak issue where `InputEvent` objects passed to input handling methods are not garbage collected properly. Without explicit disposal, these objects accumulate in memory at rates of up to 60 objects per second during active input, causing memory usage to climb and FPS to drop over time.

This rule detects when override methods for `_Input`, `_UnhandledInput`, or `_GuiInput` don't call `Dispose()` on their `InputEvent` parameters, helping prevent this well-documented memory leak.

## Background

This issue has been reported across multiple Godot versions (3.1 through 4.4) in the following GitHub issues:
- [C# _UnhandledInput(...) Memory Leak #30821](https://github.com/godotengine/godot/issues/30821)
- [Memory leak with the gui_input() signal in C# #16877](https://github.com/godotengine/godot/issues/16877)
- [C# GC not cleaning up InputEvent objects very quickly #41543](https://github.com/godotengine/godot/issues/41543)
- [Memory leak occurs when using C# programming in the `_input` function #99347](https://github.com/godotengine/godot/issues/99347)

The root cause is in Godot's C#/C++ binding layer where InputEvent objects don't get properly released to the garbage collector.

## How to fix violations

### Recommended Approach: Data Extraction Pattern

The best practice is to extract only the data you need immediately, dispose the InputEvent, then work with simple value types:

```csharp
public override void _Input(InputEvent @event)
{
// ✅ GOOD: Extract data immediately
var inputData = ExtractInputData(@event);
@event.Dispose(); // Required to prevent memory leaks

// Process with extracted data (no InputEvent references)
ProcessInput(inputData);
}

private InputData ExtractInputData(InputEvent @event)
{
return @event switch
{
InputEventKey key => new InputData
{
Type = InputType.Key,
Keycode = key.Keycode,
Pressed = key.Pressed,
Position = Vector2.Zero
},
InputEventMouse mouse => new InputData
{
Type = InputType.Mouse,
Position = mouse.Position,
Pressed = mouse.ButtonMask != 0,
Keycode = Key.None
},
_ => new InputData { Type = InputType.Other }
};
}

private struct InputData
{
public InputType Type;
public Vector2 Position;
public bool Pressed;
public Key Keycode;
}

private enum InputType { Key, Mouse, Other }
```

### Simple Fix: Direct Disposal

For simple cases, add disposal at the end of the method:

```csharp
public override void _Input(InputEvent @event)
{
if (@event is InputEventKey keyEvent && keyEvent.Pressed)
{
GD.Print($"Key pressed: {keyEvent.Keycode}");
}

@event.Dispose(); // Fix: Add disposal call
}
```

## Why This Approach Works

1. **Prevents memory leaks** through proper disposal
2. **Avoids keeping references** to disposed objects
3. **Works with multiple listeners** (each extracts their own data)
4. **Performance-friendly** (works with value types after extraction)
5. **Future-proof** (won't break if Godot fixes the underlying issue)

## When to suppress warnings

- When the `InputEvent` is passed to another method that handles disposal
- When using try-finally blocks that guarantee disposal
- In integration tests where memory leaks are acceptable

**❌ DO NOT suppress when:**
- Storing InputEvent references beyond the method scope
- Assuming garbage collection will handle cleanup
- Using caching or pooling with InputEvent objects

## Example of a violation

```csharp
public class Player : Node
{
private InputEvent _lastEvent; // ❌ BAD: Storing reference

public override void _Input(InputEvent @event) // GD0002: Missing disposal
{
_lastEvent = @event; // ❌ BAD: Reference will become invalid

if (@event is InputEventKey keyEvent && keyEvent.Pressed)
{
ProcessKeyPress(keyEvent.Keycode);
}
// ❌ Missing: @event.Dispose();
}
}
```

## Example of the fix

```csharp
public class Player : Node
{
private InputData _lastInput; // ✅ GOOD: Store extracted data

public override void _Input(InputEvent @event)
{
// ✅ GOOD: Extract data first
var inputData = ExtractInputData(@event);
_lastInput = inputData; // ✅ GOOD: Safe to store

@event.Dispose(); // ✅ GOOD: Prevent memory leak

// ✅ GOOD: Process with extracted data
if (inputData.Type == InputType.Key && inputData.Pressed)
{
ProcessKeyPress(inputData.Keycode);
}
}
}
```

## Alternative approaches

### Using try-finally blocks

```csharp
public override void _Input(InputEvent @event)
{
try
{
var inputData = ExtractInputData(@event);
ProcessInput(inputData);
}
finally
{
@event.Dispose(); // Guaranteed disposal
}
}
```

### Delegating disposal responsibility

```csharp
public override void _Input(InputEvent @event)
{
ProcessAndDisposeEvent(@event);
}

private void ProcessAndDisposeEvent(InputEvent @event)
{
var inputData = ExtractInputData(@event);
@event.Dispose(); // This method handles disposal

ProcessInput(inputData);
}
```

### Alternative disposal method

Some users report success with `Unreference()` instead of `Dispose()`:

```csharp
public override void _Input(InputEvent @event)
{
var inputData = ExtractInputData(@event);
@event.Unreference(); // Alternative to Dispose()

ProcessInput(inputData);
}
```

## What NOT to do

### ❌ Storing InputEvent references

```csharp
// ❌ BAD: InputEvent reference becomes invalid after disposal
private InputEvent _storedEvent;

public override void _Input(InputEvent @event)
{
_storedEvent = @event; // Will cause issues
@event.Dispose();
}
```

### ❌ Caching or pooling InputEvents

```csharp
// ❌ BAD: InputEvents are engine-managed, not user-created
private static readonly Queue<InputEvent> _eventPool = new();

public override void _Input(InputEvent @event)
{
_eventPool.Enqueue(@event); // Don't do this
}
```

### ❌ Manual garbage collection

```csharp
// ❌ BAD: Performance killer
public override void _Input(InputEvent @event)
{
ProcessInput(@event);
GC.Collect(); // Never do this
}
```

## Performance Impact

Without this fix, memory usage grows continuously:
- **Mouse movement**: ~60 leaked objects per second
- **Active gameplay**: 3MB leaked in 30 seconds
- **Long sessions**: Noticeable FPS drops over time

With proper disposal, memory usage remains stable throughout gameplay.

## See also

- [GD0001: Signal connection memory leak](GD0001.md)
- [Godot GitHub Issues: InputEvent Memory Leaks](https://github.com/godotengine/godot/issues?q=is%3Aissue+inputevent+memory+leak+C%23)
- [Godot Documentation: Handling input](https://docs.godotengine.org/en/stable/tutorials/inputs/handling_input_events.html)
3 changes: 2 additions & 1 deletion src/GodotSharpAnalyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
GD0001 | Memory | Warning | SignalConnectionLeak, [SignalConnectionLeakAnalyzer](Analyzers/Memory/SignalConnectionLeakAnalyzer.cs)
GD0001a | Memory | Info | SignalLambdaConnectionLeak, [SignalConnectionLeakAnalyzer](Analyzers/Memory/SignalConnectionLeakAnalyzer.cs)
GD0001a | Memory | Info | SignalLambdaConnectionLeak, [SignalConnectionLeakAnalyzer](Analyzers/Memory/SignalConnectionLeakAnalyzer.cs)
GD0002 | Memory | Warning | InputEventNotDisposed, [InputEventDisposalAnalyzer](Analyzers/Memory/InputEventDisposalAnalyzer.cs)
Loading
Loading