Skip to content

fix: Resolve race conditions and null pointer issues #100

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 15, 2025
Merged

fix: Resolve race conditions and null pointer issues #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66442b7
fix: Ensure CPlayer vtable validity before virtual calls
ELDment Nov 15, 2025
66083b8
fix: Ensure options are properly locked for thread safety
ELDment Nov 15, 2025
6ca89fc
fix: Add null check for CPlayer pointer before vtable validation
ELDment Nov 15, 2025
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
149 changes: 86 additions & 63 deletions managed/src/SwiftlyS2.Core/Modules/Menus/MenuAPI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public IReadOnlyList<IMenuOption> Options {
// private readonly ConcurrentDictionary<IPlayer, IReadOnlyList<IMenuOption>> visibleOptionsCache = new();
private readonly ConcurrentDictionary<IPlayer, CancellationTokenSource> autoCloseCancelTokens = new();

private readonly ConcurrentDictionary<IPlayer, string> renderCache = new();
// private readonly ConcurrentDictionary<IPlayer, string> renderCache = new();
private readonly CancellationTokenSource renderLoopCancellationTokenSource = new();

private volatile bool disposed;
Expand All @@ -137,18 +137,21 @@ public MenuAPI( ISwiftlyCore core, MenuConfiguration configuration, MenuKeybindO
Builder = builder;
// Parent = parent;

options.Clear();
lock (optionsLock)
{
options.Clear();
}
selectedOptionIndex.Clear();
desiredOptionIndex.Clear();
// selectedDisplayLine.Clear();
autoCloseCancelTokens.Clear();
// visibleOptionsCache.Clear();
renderCache.Clear();
// renderCache.Clear();

maxOptions = 0;
// maxDisplayLines = 0;

core.Event.OnTick += OnTick;
// core.Event.OnTick += OnTick;

_ = Task.Run(async () =>
{
Expand Down Expand Up @@ -200,19 +203,22 @@ public void Dispose()
}
});

// options.ForEach(option => option.Dispose());
options.Clear();
lock (optionsLock)
{
// options.ForEach(option => option.Dispose());
options.Clear();
}
selectedOptionIndex.Clear();
desiredOptionIndex.Clear();
// selectedDisplayLine.Clear();
autoCloseCancelTokens.Clear();
// visibleOptionsCache.Clear();
renderCache.Clear();
// renderCache.Clear();

maxOptions = 0;
// maxDisplayLines = 0;

core.Event.OnTick -= OnTick;
// core.Event.OnTick -= OnTick;

renderLoopCancellationTokenSource?.Cancel();
renderLoopCancellationTokenSource?.Dispose();
Expand All @@ -221,24 +227,24 @@ public void Dispose()
GC.SuppressFinalize(this);
}

private void OnTick()
{
if (maxOptions <= 0)
{
return;
}
// private void OnTick()
// {
// if (maxOptions <= 0)
// {
// return;
// }

foreach (var kvp in renderCache)
{
var player = kvp.Key;
if (!player.IsValid || player.IsFakeClient)
{
continue;
}
// foreach (var kvp in renderCache)
// {
// var player = kvp.Key;
// if (!player.IsValid || player.IsFakeClient)
// {
// continue;
// }

NativePlayer.SetCenterMenuRender(player.PlayerID, kvp.Value);
}
}
// NativePlayer.SetCenterMenuRender(player.PlayerID, kvp.Value);
// }
// }

private void OnRender()
{
Expand All @@ -264,22 +270,25 @@ private void OnRender()
: Math.Clamp(baseMaxVisibleItems, 1, 5);
var halfVisible = maxVisibleItems / 2;

lock (optionsLock)
foreach (var (player, desiredIndex, selectedIndex) in playerStates)
{
foreach (var (player, desiredIndex, selectedIndex) in playerStates)
{
ProcessPlayerMenu(player, desiredIndex, selectedIndex, maxOptions, maxVisibleItems, halfVisible);
}
ProcessPlayerMenu(player, desiredIndex, selectedIndex, maxOptions, maxVisibleItems, halfVisible);
}
}

private void ProcessPlayerMenu( IPlayer player, int desiredIndex, int selectedIndex, int maxOptions, int maxVisibleItems, int halfVisible )
{
var filteredOptions = options.Where(opt => opt.Visible && opt.GetVisible(player)).ToList();
if (filteredOptions.Count == 0)
var filteredOptions = new List<IMenuOption>();
lock (optionsLock)
{
filteredOptions = options.Where(opt => opt.Visible && opt.GetVisible(player)).ToList();
}

if (filteredOptions.Count <= 0)
{
var emptyHtml = BuildMenuHtml(player, [], 0, 0, maxOptions, maxVisibleItems);
_ = renderCache.AddOrUpdate(player, emptyHtml, ( _, _ ) => emptyHtml);
// _ = renderCache.AddOrUpdate(player, emptyHtml, ( _, _ ) => emptyHtml);
core.Scheduler.NextTick(() => NativePlayer.SetCenterMenuRender(player.PlayerID, emptyHtml));
return;
}

Expand All @@ -293,33 +302,40 @@ private void ProcessPlayerMenu( IPlayer player, int desiredIndex, int selectedIn
});

var html = BuildMenuHtml(player, visibleOptions, safeArrowPosition, clampedDesiredIndex, maxOptions, maxVisibleItems);
_ = renderCache.AddOrUpdate(player, html, ( _, _ ) => html);

var currentOption = visibleOptions[safeArrowPosition];
var currentOriginalIndex = options.IndexOf(currentOption);
// _ = renderCache.AddOrUpdate(player, html, ( _, _ ) => html);
core.Scheduler.NextTick(() => NativePlayer.SetCenterMenuRender(player.PlayerID, html));

if (currentOriginalIndex != selectedIndex)
lock (optionsLock)
{
var updateResult = selectedOptionIndex.TryUpdate(player, currentOriginalIndex, selectedIndex);
if (updateResult && currentOriginalIndex != desiredIndex)
var currentOriginalIndex = options.IndexOf(visibleOptions[safeArrowPosition]);

if (currentOriginalIndex != selectedIndex)
{
_ = desiredOptionIndex.TryUpdate(player, currentOriginalIndex, desiredIndex);
var updateResult = selectedOptionIndex.TryUpdate(player, currentOriginalIndex, selectedIndex);
if (updateResult && currentOriginalIndex != desiredIndex)
{
_ = desiredOptionIndex.TryUpdate(player, currentOriginalIndex, desiredIndex);
}
}
}
}

private (IReadOnlyList<IMenuOption> VisibleOptions, int ArrowPosition) GetVisibleOptionsAndArrowPosition( List<IMenuOption> filteredOptions, int clampedDesiredIndex, int maxVisibleItems, int halfVisible )
{
var filteredMaxOptions = filteredOptions.Count;
var desiredOption = options[clampedDesiredIndex];
var mappedDesiredIndex = filteredOptions.IndexOf(desiredOption);

if (mappedDesiredIndex < 0)
var filteredMaxOptions = -1;
var mappedDesiredIndex = -1;
lock (optionsLock)
{
mappedDesiredIndex = filteredOptions
.Select(( opt, idx ) => (Index: idx, Distance: Math.Abs(options.IndexOf(opt) - clampedDesiredIndex)))
.MinBy(x => x.Distance)
.Index;
filteredMaxOptions = filteredOptions.Count;
mappedDesiredIndex = filteredOptions.IndexOf(options[clampedDesiredIndex]);

if (mappedDesiredIndex < 0)
{
mappedDesiredIndex = filteredOptions
.Select(( opt, idx ) => (Index: idx, Distance: Math.Abs(options.IndexOf(opt) - clampedDesiredIndex)))
.MinBy(x => x.Distance)
.Index;
}
}

if (filteredMaxOptions <= maxVisibleItems)
Expand Down Expand Up @@ -467,7 +483,7 @@ public void HideForPlayer( IPlayer player )

SetFreezeState(player, false);

_ = renderCache.TryRemove(player, out _);
// _ = renderCache.TryRemove(player, out _);

if (autoCloseCancelTokens.TryRemove(player, out var token))
{
Expand Down Expand Up @@ -521,25 +537,29 @@ public bool RemoveOption( IMenuOption option )

public bool MoveToOption( IPlayer player, IMenuOption option )
{
return MoveToOptionIndex(player, options.IndexOf(option));
lock (optionsLock)
{
return MoveToOptionIndex(player, options.IndexOf(option));
}
}

public bool MoveToOptionIndex( IPlayer player, int index )
{
lock (optionsLock)

if (maxOptions == 0 || !desiredOptionIndex.TryGetValue(player, out var oldIndex))
{
if (maxOptions == 0 || !desiredOptionIndex.TryGetValue(player, out var oldIndex))
{
return false;
}
return false;
}

var targetIndex = ((index % maxOptions) + maxOptions) % maxOptions;
var direction = Math.Sign(targetIndex - oldIndex);
if (direction == 0)
{
return true;
}
var targetIndex = ((index % maxOptions) + maxOptions) % maxOptions;
var direction = Math.Sign(targetIndex - oldIndex);
if (direction == 0)
{
return true;
}

lock (optionsLock)
{
var visibleIndex = Enumerable.Range(0, maxOptions)
.Select(i => (((targetIndex + (i * direction)) % maxOptions) + maxOptions) % maxOptions)
.FirstOrDefault(idx => options[idx].Visible && options[idx].GetVisible(player), -1);
Expand All @@ -550,7 +570,10 @@ public bool MoveToOptionIndex( IPlayer player, int index )

public IMenuOption? GetCurrentOption( IPlayer player )
{
return selectedOptionIndex.TryGetValue(player, out var index) ? options[index] : null;
lock (optionsLock)
{
return selectedOptionIndex.TryGetValue(player, out var index) ? options[index] : null;
}
}

public int GetCurrentOptionIndex( IPlayer player )
Expand Down
Loading
Loading