vladfaust / unity-wakatime

WakaTime plugin for Unity ⏱
MIT License
120 stars 27 forks source link

Native Collection has not been disposed, resulting in a memory leak. #29

Open NachtgeistW opened 3 years ago

NachtgeistW commented 3 years ago

This happened in 2021.1. Every time I press "Play" button, they appear in console. I used com.unity.jobs and caught these:

> A Native Collection has not been disposed, resulting in a memory leak. Allocated from: > Unity.Collections.NativeArray`1:.ctor(Byte[], Allocator) > UnityEngine.Networking.UploadHandlerRaw:.ctor(Byte[]) > WakaTime.Plugin:SendHeartbeat(Boolean) (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:145) > WakaTime.Plugin:OnSceneClosing(Scene, Boolean) (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:205) > UnityEditor.SceneManagement.EditorSceneManager:Internal_SceneClosing(Scene, Boolean) > A Native Collection has not been disposed, resulting in a memory leak. Allocated from: > Unity.Collections.NativeArray`1:.ctor(Byte[], Allocator) > UnityEngine.Networking.UploadHandlerRaw:.ctor(Byte[]) > WakaTime.Plugin:SendHeartbeat(Boolean) (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:145) > WakaTime.Plugin:Initialize() (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:62) > WakaTime.Plugin:OnScriptReload() (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:181) > A Native Collection has not been disposed, resulting in a memory leak. Allocated from: > Unity.Collections.NativeArray`1:.ctor(Byte[], Allocator) > UnityEngine.Networking.UploadHandlerRaw:.ctor(Byte[]) > WakaTime.Plugin:SendHeartbeat(Boolean) (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:145) > WakaTime.Plugin:OnPlaymodeStateChanged(PlayModeStateChange) (at Library\PackageCache\com.vladfaust.unitywakatime@4dedbeac2e\Editor\Plugin.cs:185) > UnityEditor.EditorApplication:Internal_PlayModeStateChanged(PlayModeStateChange) > UnityEditor.EditorApplication:set_isPlaying(Boolean) > Unity.Entities.Editor.LiveLinkToolbar:TogglePlaying() (at Library\PackageCache\com.unity.entities@0.17.0-preview.41\Unity.Entities.Editor\LiveLink\LiveLinkToolbar.cs:94) > Unity.Entities.Editor.LiveLinkToolbar:DrawPlaybar(CommandExecuteContext) (at Library\PackageCache\com.unity.entities@0.17.0-preview.41\Unity.Entities.Editor\LiveLink\LiveLinkToolbar.cs:60) > UnityEditor.CommandService:ExecuteCommand(String, CommandHint, Object[]) > UnityEditor.CommandService:Execute(String, CommandHint, Object[]) > UnityEditor.ModeService:Execute(String, CommandHint, Object[]) > UnityEditor.ModeService:Execute(String, Object[]) > UnityEditor.Toolbars.PlayModeButtons:OverrideGUIHandler() > UnityEngine.UIElements.IMGUIContainer:DoOnGUI(Event, Matrix4x4, Rect, Boolean, Rect, Action, Boolean) > UnityEngine.UIElements.IMGUIContainer:HandleIMGUIEvent(Event, Matrix4x4, Rect, Action, Boolean) > UnityEngine.UIElements.IMGUIContainer:HandleIMGUIEvent(Event, Action, Boolean) > UnityEngine.UIElements.IMGUIContainer:HandleIMGUIEvent(Event, Boolean) > UnityEngine.UIElements.IMGUIContainer:SendEventToIMGUIRaw(EventBase, Boolean, Boolean) > UnityEngine.UIElements.IMGUIContainer:SendEventToIMGUI(EventBase, Boolean, Boolean) > UnityEngine.UIElements.IMGUIContainer:HandleEvent(EventBase) > UnityEngine.UIElements.CallbackEventHandler:HandleEventAtTargetPhase(EventBase) > UnityEngine.UIElements.MouseCaptureDispatchingStrategy:DispatchEvent(EventBase, IPanel) > UnityEngine.UIElements.EventDispatcher:ApplyDispatchingStrategies(EventBase, IPanel, Boolean) > UnityEngine.UIElements.EventDispatcher:ProcessEvent(EventBase, IPanel) > UnityEngine.UIElements.EventDispatcher:ProcessEventQueue() > UnityEngine.UIElements.EventDispatcher:OpenGate() > UnityEngine.UIElements.EventDispatcherGate:Dispose() > UnityEngine.UIElements.EventDispatcher:ProcessEvent(EventBase, IPanel) > UnityEngine.UIElements.EventDispatcher:Dispatch(EventBase, IPanel, DispatchMode) > UnityEngine.UIElements.BaseVisualElementPanel:SendEvent(EventBase, DispatchMode) > UnityEngine.UIElements.UIElementsUtility:DoDispatch(BaseVisualElementPanel) > UnityEngine.UIElements.UIElementsUtility:UnityEngine.UIElements.IUIElementsUtility.ProcessEvent(Int32, IntPtr, Boolean&) > UnityEngine.UIElements.UIEventRegistration:ProcessEvent(Int32, IntPtr) > UnityEngine.UIElements.<>c:<.cctor>b__1_2(Int32, IntPtr) > UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&)
HaipaDev commented 3 years ago

Same here, its leaking memory

vladfaust commented 3 years ago

I can guess that UploadHandlerRaw shall be disposed properly @ https://github.com/vladfaust/unity-wakatime/blob/72a78455940c4585bb2ed3fe491527898adf4148/Assets/com.vladfaust.unitywakatime/Editor/Plugin.cs#L145

JayPeet commented 2 years ago

I think the UnityWebRequest.Post needs to be wrapped in a using statement so that it disposes correctly have Dispose called manually in the completed callback.

I am pretty sure the native collection leak is something which only happens on more recent unity versions, as it is not a problem on pre 2021 versions from what I saw

I can go make a PR with fixes in if needed :)

JayPeet commented 2 years ago

PR is here: https://github.com/vladfaust/unity-wakatime/pull/31

BrianLDev commented 2 years ago

This bug is still happening and causing constant error messages in Unity. Can someone approve the PR that JayPeet created to fix this?

vladfaust commented 2 years ago

@BrianLDev

Actually, this sometimes still has the issue.

BrianLDev commented 2 years ago

Ah that's a shame. The error messages were getting annoying and I was hoping this would fix it.

JayPeet commented 2 years ago

Yeah, I need to investigate it properly still. Just been too busy :(

vladfaust commented 2 years ago

I'm not a Unity developer anymore. At least for now. Would be happy if someone takes over this repository. @Hermesiss?

Hermesiss commented 2 years ago

Yes, I'm still an active Unity developer. @vladfaust can you transfer this repo to me?

vladfaust commented 2 years ago

@Hermesiss let's begin with fixing this issue (you're a collaborator) 😃

Hermesiss commented 2 years ago

@JayPeet @NachtgeistW @HyperGamesDev Can you provide an example project? I can't reproduce this error

HaipaDev commented 2 years ago

my repository SSS222 had WakaTime before, sorry for the inconvenience but you'd have to add it once again now as I removed it(also the Jobs addon). Not sure when it appeared though so it may be difficult to reproduce

HermesZum commented 2 years ago

It's about Unity Engine

public UnityWebRequestAsyncOperation SendWebRequest() in class UnityWebRequest Begin communicating with the remote server. External documentation for UnityWebRequest.SendWebRequest

If this operation can be replaced for other the problem will be solved.

for example, if you use the obsolete: Method 'UnityEngine.Networking.UnityWebRequest.Send' is obsolete: Use SendWebRequest. It returns a UnityWebRequestAsyncOperation which contains a reference to the WebRequest object.

The leak disapear.

hs-pp commented 2 years ago

Hello! Curious if this has been investigated yet.

HermesZum commented 2 years ago

I think I managed to reduce the number of warnings.

But the

static void OnPlaymodeStateChanged(PlayModeStateChange change) {
      SendHeartbeat();
    }

still throws an error when the editor begin the Play, but not always.

If you test this snippet, tell me how it went, so that it can be improved:

Plugin.cs

static void SendHeartbeat(bool fromSave = false) 
    {
      var currentScene = EditorSceneManager.GetActiveScene().path;
      var file = currentScene != string.Empty
        ? Application.dataPath + "/" + currentScene.Substring("Assets/".Length)
        : string.Empty;
      var heartbeat = new Heartbeat(file, fromSave);
      var heartbeatJSON = JsonUtility.ToJson(heartbeat);
      var request = UnityWebRequest.Post(URL_PREFIX + "users/current/heartbeats?api_key=" + _apiKey, string.Empty);
      request.uploadHandler = new UploadHandlerRaw(System.Text.Encoding.UTF8.GetBytes(heartbeatJSON));
      request.SetRequestHeader("Content-Type", "application/json");

      request.SendWebRequest().completed +=
        operation => 
        {
          if (_debug) Debug.Log("<WakaTime> Sending heartbeat...");

          if ((heartbeat.time - _lastHeartbeat.time < HEARTBEAT_COOLDOWN) && !fromSave &&
              (heartbeat.entity == _lastHeartbeat.entity)) 
          {
            if (_debug) Debug.Log("<WakaTime> Skip this heartbeat");
            request.Dispose();
            return;
          }

          if (request.downloadHandler.text == string.Empty) 
          {
            Debug.LogWarning(
              "<WakaTime> Network is unreachable. Consider disabling completely if you're working offline");
            request.Dispose();
            return;
          }

          if (_debug)
            Debug.Log("<WakaTime> Got response\n" + request.downloadHandler.text);

          var response =
            JsonUtility.FromJson<Response<HeartbeatResponse>>(
              request.downloadHandler.text);

          if (response.error != null) 
          {
            if (response.error == "Duplicate") 
            {
              if (_debug) Debug.LogWarning("<WakaTime> Duplicate heartbeat");
              request.Dispose();
            }
            else 
            {
              Debug.LogError(
                "<WakaTime> Failed to send heartbeat to WakaTime!\n" +
                response.error);
              request.Dispose();
            }
          }
          else 
          {
            if (_debug) Debug.Log("<WakaTime> Sent heartbeat!");
            _lastHeartbeat = response.data;
            request.Dispose();
          }
        };
    }
celojevic commented 2 years ago

The easiest fix i found was to go to the PlayFabEditorHttp script, go the the Post coroutine method, and add www.Dispose(); at the end of the method

vanBassum commented 2 years ago

Since this problem still isn't fixed, I decided to create my own plugin. Used this one as example, but I started from zero in order to create better readable code. At the risk of being flamed for shameless advertising, https://github.com/vanBassum/Wakatime.Unity :)

Please give it a try, and let me know if anything needs tweaking.

alanhamlett commented 2 years ago

I've added your repo to the Unity page on WakaTime's website:

https://wakatime.com/unity

vanBassum commented 2 years ago

Awesome, thanks

NachtgeistW commented 2 years ago

Since this problem still isn't fixed, I decided to create my own plugin. Used this one as example, but I started from zero in order to create better readable code. At the risk of being flamed for shameless advertising, https://github.com/vanBassum/Wakatime.Unity :)

Please give it a try, and let me know if anything needs tweaking.

Thank you!

BrianLDev commented 2 years ago

@alanhamlett Can we remove the link to vladfaust's version of unity wakatime from Wakatime's website? This bug has been unresolved for 1.5 years and there are better alternatives without the bug. Or at least put the link at the bottom of the list with a note that it has unresolved bugs.

MelvinKuhlmann commented 7 months ago

Bump? This still seems to happen in 2023.2.3f1