ToxicMushroom/Penumbra
1
0
Fork 0
mirror of https://github.com/xivdev/Penumbra.git synced 2026-01-03 06:13:45 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Skin Fixer: Fix potential ref leak + add SRH

Browse source 
`SafeResourceHandle` wraps a `ResourceHandle*` with auto `IncRef` / `DecRef`, to further help prevent leaks.
This commit is contained in:
Exter-N 2023-08-30 19:16:22 +02:00
parent ec14efb789
commit f238049750
2 changed files with 60 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

34
Penumbra/Interop/SafeHandles/SafeResourceHandle.cs Normal file
View file

@ -0,0 +1,34 @@
using System;
using System.Runtime.InteropServices;
using System.Threading;
using FFXIVClientStructs.FFXIV.Client.System.Resource.Handle;
namespace Penumbra.Interop.SafeHandles;
public unsafe class SafeResourceHandle : SafeHandle
{
public ResourceHandle* ResourceHandle => (ResourceHandle*)handle;
public override bool IsInvalid => handle == 0;
public SafeResourceHandle(ResourceHandle* handle, bool incRef, bool ownsHandle = true) : base(0, ownsHandle)
{
if (incRef && !ownsHandle)
throw new ArgumentException("Non-owning SafeResourceHandle with IncRef is unsupported");
if (incRef && handle != null)
handle->IncRef();
SetHandle((nint)handle);
}
public static SafeResourceHandle CreateInvalid()
=> new(null, incRef: false);
protected override bool ReleaseHandle()
{
var handle = Interlocked.Exchange(ref this.handle, 0);
if (handle != 0)
((ResourceHandle*)handle)->DecRef();
return true;
}
}

40
Penumbra/Interop/Services/SkinFixer.cs
View file

@ -14,6 +14,7 @@ using Penumbra.GameData;
using Penumbra.GameData.Enums;
using Penumbra.Interop.PathResolving;
using Penumbra.Interop.ResourceLoading;
using Penumbra.Interop.SafeHandles;
using Penumbra.String.Classes;
namespace Penumbra.Interop.Services;
@ -44,7 +45,7 @@ public unsafe class SkinFixer : IDisposable
private readonly ResourceLoader _resources;
private readonly CharacterUtility _utility;
private readonly ConcurrentDictionary<nint /* CharacterBase* */, nint /* ResourceHandle* */> _skinShpks = new();
private readonly ConcurrentDictionary<nint /* CharacterBase* */, SafeResourceHandle> _skinShpks = new();
private readonly object _lock = new();
@ -89,8 +90,7 @@ public unsafe class SkinFixer : IDisposable
_gameEvents.CharacterBaseCreated -= OnCharacterBaseCreated;
_gameEvents.CharacterBaseDestructor -= OnCharacterBaseDestructor;
foreach (var skinShpk in _skinShpks.Values)
if (skinShpk != nint.Zero)
((ResourceHandle*)skinShpk)->DecRef();
skinShpk.Dispose();
_skinShpks.Clear();
_moddedSkinShpkCount = 0;
}
@ -105,29 +105,41 @@ public unsafe class SkinFixer : IDisposable
Task.Run(delegate
{
nint skinShpk;
var skinShpk = SafeResourceHandle.CreateInvalid();
try
{
var data = _collectionResolver.IdentifyCollection((DrawObject*)drawObject, true);
skinShpk = data.Valid ? (nint)_resources.LoadResolvedResource(ResourceCategory.Shader, ResourceType.Shpk, SkinShpkPath.Path, data) : nint.Zero;
if (data.Valid)
{
var loadedShpk = _resources.LoadResolvedResource(ResourceCategory.Shader, ResourceType.Shpk, SkinShpkPath.Path, data);
skinShpk = new SafeResourceHandle((ResourceHandle*)loadedShpk, incRef: false);
}
}
catch (Exception e)
{
Penumbra.Log.Error($"Error while resolving skin.shpk for human {drawObject:X}: {e}");
skinShpk = nint.Zero;
}
if (skinShpk != nint.Zero && _skinShpks.TryAdd(drawObject, skinShpk) && skinShpk != _utility.DefaultSkinShpkResource)
Interlocked.Increment(ref _moddedSkinShpkCount);
if (!skinShpk.IsInvalid)
{
if (_skinShpks.TryAdd(drawObject, skinShpk))
{
if ((nint)skinShpk.ResourceHandle != _utility.DefaultSkinShpkResource)
Interlocked.Increment(ref _moddedSkinShpkCount);
}
else
skinShpk.Dispose();
}
});
}
private void OnCharacterBaseDestructor(nint characterBase)
{
if (_skinShpks.Remove(characterBase, out var skinShpk) && skinShpk != nint.Zero)
if (_skinShpks.Remove(characterBase, out var skinShpk))
{
((ResourceHandle*)skinShpk)->DecRef();
if (skinShpk != _utility.DefaultSkinShpkResource)
var handle = skinShpk.ResourceHandle;
skinShpk.Dispose();
if ((nint)handle != _utility.DefaultSkinShpkResource)
Interlocked.Decrement(ref _moddedSkinShpkCount);
}
}
@ -136,12 +148,12 @@ public unsafe class SkinFixer : IDisposable
{
if (!_enabled || // Can be toggled on the debug tab.
_moddedSkinShpkCount == 0 || // If we don't have any on-screen instances of modded skin.shpk, we don't need the slow path at all.
!_skinShpks.TryGetValue(human, out var skinShpk) || skinShpk == nint.Zero)
!_skinShpks.TryGetValue(human, out var skinShpk))
return _onRenderMaterialHook!.Original(human, param);
var material = param->Model->Materials[param->MaterialIndex];
var shpkResource = ((Structs.MtrlResource*)material->MaterialResourceHandle)->ShpkResourceHandle;
if ((nint)shpkResource != skinShpk)
if ((nint)shpkResource != (nint)skinShpk.ResourceHandle)
return _onRenderMaterialHook!.Original(human, param);
Interlocked.Increment(ref _slowPathCallDelta);
@ -154,7 +166,7 @@ public unsafe class SkinFixer : IDisposable
lock (_lock)
try
{
_utility.Address->SkinShpkResource = (Structs.ResourceHandle*)skinShpk;
_utility.Address->SkinShpkResource = (Structs.ResourceHandle*)skinShpk.ResourceHandle;
return _onRenderMaterialHook!.Original(human, param);
}
finally