TextureFromFile leak post-mortem

DOC.md

TextureFromFile leak post-mortem

Usages of TextureFromFile in the game to load an image from StreamingAssets could lead to a memory leak of the loaded texture in the case of the TextureFromFile instance being cancelled before it finished.

This would happen in the case of the saga map collection view object reuse (e.g. SagaMapPlayStoryPolaroid) where a view object would be loaded and an initial TextureFromFile instance would get created, and then in the same frame that view object would be recycled and reused for a different section of the map, causing that original TextureFromFile instance to be cancelled.

The TextureFromFile.Cancel method looked like this:

public void Cancel()
{
    if (!_isRunning) return;

    _unityWebRequest.Abort();
}

It turns out, combining UnityWebRequest.Abort with the way TextureFromFile.TextureFromFilePathRoutine was implemented below leads to a memory leak:

private IEnumerator<float> TextureFromFilePathRoutine(IPendingPromise<Texture2D> promise) {
  using (_unityWebRequest = UnityWebRequestTexture.GetTexture($"file://{FilePath}"))
  {
      UnityWebRequestAsyncOperation asyncOperation = _unityWebRequest.SendWebRequest();

      _isRunning = true;

      yield return Timing.WaitUntilDone(asyncOperation);
      
      if (_unityWebRequest.isNetworkError || _unityWebRequest.isHttpError) {
          promise.Reject(new Exception($"TextureFromFile.TextureFromFilePathRoutine() failed to load with error: {_unityWebRequest.error}"));
      }
      else {
          Texture2D texture = DownloadHandlerTexture.GetContent(_unityWebRequest);

          if (texture == null)
          {
              string errorMessage = $"Texture failed to create for {FilePath}: {_unityWebRequest.error}";
              Exception exception = new Exception(errorMessage);
              promise.Reject(exception);
              yield break;
          }
          
          texture.name = Path.GetFileName(FilePath);
          
          promise.Resolve(texture);
      }

      _isRunning = false;
  }
}

Simply moving the Texture2D texture = DownloadHandlerTexture.GetContent(_unityWebRequest); line to just below the yield return Timing.WaitUntilDone(asyncOperation); like the below code does not fix the issue:

private IEnumerator<float> TextureFromFilePathRoutine(IPendingPromise<Texture2D> promise) {
  using (_unityWebRequest = UnityWebRequestTexture.GetTexture($"file://{FilePath}"))
  {
      UnityWebRequestAsyncOperation asyncOperation = _unityWebRequest.SendWebRequest();

      _isRunning = true;

      yield return Timing.WaitUntilDone(asyncOperation);

      Texture2D texture = DownloadHandlerTexture.GetContent(_unityWebRequest);
      
      if (_unityWebRequest.isNetworkError || _unityWebRequest.isHttpError) {
          promise.Reject(new Exception($"TextureFromFile.TextureFromFilePathRoutine() failed to load with error: {_unityWebRequest.error}"));
      }
      else {
          if (texture == null)
          {
              string errorMessage = $"Texture failed to create for {FilePath}: {_unityWebRequest.error}";
              Exception exception = new Exception(errorMessage);
              promise.Reject(exception);
              yield break;
          }
          
          texture.name = Path.GetFileName(FilePath);
          
          promise.Resolve(texture);
      }

      _isRunning = false;
  }
}

To fix the memory leak, I had to replace the use of UnityWebRequest.Abort with using an _isCancelled flag, and make sure to always call Texture2D texture = DownloadHandlerTexture.GetContent(_unityWebRequest); even if the process had been cancelled. The final, non-leaking code is below:

public void Cancel()
{
    if (!_isRunning) return;

    _isCancelled = true;
}

private IEnumerator<float> TextureFromFilePathRoutine(IPendingPromise<Texture2D> promise) {
    using (_unityWebRequest = UnityWebRequestTexture.GetTexture($"file://{FilePath}"))
    {
        UnityWebRequestAsyncOperation asyncOperation = _unityWebRequest.SendWebRequest();

        _isRunning = true;

        yield return Timing.WaitUntilDone(asyncOperation);
        
        Texture2D texture = DownloadHandlerTexture.GetContent(_unityWebRequest);

        if (_isCancelled)
        {
            promise.Reject(new Exception($"TextureFromFile.TextureFromFilePathRoutine() cancelled before it could complete loading the texture"));   
        }
        else if (_unityWebRequest.isNetworkError || _unityWebRequest.isHttpError) {
            promise.Reject(new Exception($"TextureFromFile.TextureFromFilePathRoutine() failed to load with error: {_unityWebRequest.error}"));
        }
        else {
            if (texture == null)
            {
                string errorMessage = $"Texture failed to create for {FilePath}: {_unityWebRequest.error}";
                Exception exception = new Exception(errorMessage);
                promise.Reject(exception);
                yield break;
            }
            
            texture.name = Path.GetFileName(FilePath);
            
            promise.Resolve(texture);
        }

        _isRunning = false;
    }
}