fix: set initial refcount inside lock to prevent race condition with cleanup code when loading from a task

This commit is contained in:
goat 2023-08-09 01:42:05 +02:00
parent 39e389080c
commit d1fad810cf
No known key found for this signature in database
GPG key ID: 49E2AA8C6A76498B

View file

@ -239,22 +239,31 @@ internal class TextureManager : IDisposable, IServiceType, ITextureSubstitutionP
/// If false, exceptions will be caught and a dummy texture will be returned to prevent plugins from using invalid texture handles. /// If false, exceptions will be caught and a dummy texture will be returned to prevent plugins from using invalid texture handles.
/// </param> /// </param>
/// <returns>Info object storing texture metadata.</returns> /// <returns>Info object storing texture metadata.</returns>
internal TextureInfo GetInfo(string path, bool refresh = true, bool rethrow = false) internal TextureInfo? GetInfo(string path, bool refresh = true, bool rethrow = false)
{ {
TextureInfo? info; TextureInfo? info;
lock (this.activeTextures) lock (this.activeTextures)
{ {
this.activeTextures.TryGetValue(path, out info); if (!this.activeTextures.TryGetValue(path, out info))
}
if (info == null)
{
info = new TextureInfo();
lock (this.activeTextures)
{ {
if (!this.activeTextures.TryAdd(path, info)) Debug.Assert(rethrow, "This should never run when getting outside of creator");
Log.Warning("Texture {Path} tracked twice", path);
if (!refresh)
return null;
// NOTE: We need to init the refcount here while locking the collection!
// Otherwise, if this is loaded from a task, cleanup might already try to delete it
// before it can be increased.
info = new TextureInfo
{
RefCount = 1,
};
this.activeTextures.Add(path, info);
} }
if (info == null)
throw new Exception("null info in activeTextures");
} }
if (refresh && info.KeepAliveCount == 0) if (refresh && info.KeepAliveCount == 0)
@ -353,6 +362,14 @@ internal class TextureManager : IDisposable, IServiceType, ITextureSubstitutionP
internal void NotifyTextureDisposed(string path, bool keepAlive) internal void NotifyTextureDisposed(string path, bool keepAlive)
{ {
var info = this.GetInfo(path, false); var info = this.GetInfo(path, false);
// This texture was already disposed
if (info == null)
{
Log.Warning("Disposing unknown texture {Path}", path);
return;
}
info.RefCount--; info.RefCount--;
if (keepAlive) if (keepAlive)
@ -378,8 +395,7 @@ internal class TextureManager : IDisposable, IServiceType, ITextureSubstitutionP
{ {
// This will create the texture. // This will create the texture.
// That's fine, it's probably used immediately and this will let the plugin catch load errors. // That's fine, it's probably used immediately and this will let the plugin catch load errors.
var info = this.GetInfo(path, rethrow: true); var info = this.GetInfo(path, rethrow: true)!;
info.RefCount++;
if (keepAlive) if (keepAlive)
info.KeepAliveCount++; info.KeepAliveCount++;
@ -575,7 +591,7 @@ internal class TextureManagerTextureWrap : IDalamudTextureWrap
/// <inheritdoc/> /// <inheritdoc/>
public IntPtr ImGuiHandle => !this.IsDisposed ? public IntPtr ImGuiHandle => !this.IsDisposed ?
this.manager.GetInfo(this.path).Wrap!.ImGuiHandle : this.manager.GetInfo(this.path)!.Wrap!.ImGuiHandle :
throw new InvalidOperationException("Texture already disposed. You may not render it."); throw new InvalidOperationException("Texture already disposed. You may not render it.");
/// <inheritdoc/> /// <inheritdoc/>