bryanhitc / lcu-sharp

An API wrapper for the League of Legends client.
MIT License
44 stars 10 forks source link

Stack Overflow Exception while the league client is starting #16

Closed fbrusch-ik closed 3 years ago

fbrusch-ik commented 4 years ago

Hey, it takes quite a few steps to reproduce but it should still be fairly common

  1. Connect to League via ConnectAsync.
  2. Add a Disconnect Event Handler to the API
  3. In the Disconnect Handler initiate ConnectAsync or ReconnectAsync
  4. Close the League Client
  5. Open the League Client

In the first few seconds where the client isnt fully ready EnsureConnectionAsync will then spam the LCU with requests, it still connects but a short time after it will succumb to stack overflow.

I put a bandaid fix on this by increasing the delay on exception to 5000ms, which does work but its just that: a bandaid.

bryanhitc commented 4 years ago

I won't be able to tackle this for quite some time because I'm extremely busy with school, but if you want to (or anyone else), feel free to do so.

bryanhitc commented 4 years ago

Hey @insaneKane, I realize it's been a while, but I finally have some time to take a look into this! I tried reproducing your error, but I haven't been successful. Here's the code that I'm using:

public class Program
{
    private static readonly TaskCompletionSource<bool> _work = new TaskCompletionSource<bool>(false);

    public static async Task Main(string[] args)
    {
        var api = await ConnectToClient();
        api.Disconnected += async (e, a) =>
        {
            Console.WriteLine("Disconnected! Attempting to reconnect...");
            await api.ReconnectAsync();
            Console.WriteLine("Reconnected!");
        };

        // prevent console application from ending
        await _work.Task;
    }

    private static async Task<LeagueClientApi> ConnectToClient()
    {
        Console.WriteLine("Connecting to league client...");

        var stopwatch = Stopwatch.StartNew();
        var api = await LeagueClientApi.ConnectAsync();
        stopwatch.Stop();

        Console.WriteLine($"Connected to league client ({stopwatch.ElapsedMilliseconds} ms).");
        return api;
    }
}

The output is the following:

Connecting to league client...
Connected to league client (421 ms).
*I close the league client*
Disconnected! Attempting to reconnect...
*I open the league client*
Reconnected!

If you're still experiencing this issue, then I'd greatly appreciate it if you could post a code snippet. Thanks!

bryanhitc commented 4 years ago

Unable to reproduce issue - closed.

mentorfloat commented 3 years ago

I also experienced the described issue a few times, randomly. App sometimes threw System.StackOverflowException when League client is starting up (Client takes about 5 seconds on my computer to load before the Play button is clickable). Can confirm that updating the delay to 5000ms solved this.

bryanhitc commented 3 years ago

Thanks for the heads up, @mentorfloat; I'll try to find some time to investigate this further. If you have any other information about how to reproduce this issue, then please let me know!

mentorfloat commented 3 years ago

It just says this An unhandled exception of type 'System.StackOverflowException' occurred in System.Net.Security.dll.

Initially I thought the issue is gone completely after updating the delay, but by chance while I was dragging the app window today with DragMove() when League client is starting, it threw the same exception. After a few tries, I concluded that whenever an UIElement is interacted, like when selecting a ComboBox drop down, DragMove, etc, the app will crash if it tries to connect to League LCU.

This is with the suggested 5000ms delay.

bryanhitc commented 3 years ago

@mentorfloat Do you mind showing me what your code looks like? I'm still having trouble reproducing this. Feel free to use something like Pastebin if it won't fit in a comment here.

mentorfloat commented 3 years ago

@bryanhitc doing this on a packaged app will result in the below, while doing it in debug will just result in stack overflow. "80004005" thrown by "System.Diagnostics.ProcessModuleCollection GetModules(Int32, Boolean)": "Only part of a ReadProcessMemory or WriteProcessMemory request was completed."

The way I connect to LCU is:

private async void Window_Loaded(object sender, RoutedEventArgs e) 
{
    var tasklist = new List<Task>();
    tasklist.Add(Task.Run(async() => await ConnectLCU()));
    //... simplified
    await Task.WhenAll(tasklist);
}

public async Task ConnectLCU()
{
    api = await LeagueClientApi.ConnectAsync();

    GameFlowChanged += OnGameFlowChanged;
    ChampSelectChanged += async (object sender, LeagueEvent e) => await OnChampSelect(e, api);
    ChampSelectSFXChanged += async (object sender, LeagueEvent e) => await OnSFXChanged(e, api);
    api.EventHandler.Subscribe("/lol-gameflow/v1/gameflow-phase", GameFlowChanged);
    api.EventHandler.Subscribe("/lol-champ-select/v1/session", ChampSelectChanged);
    api.EventHandler.Subscribe("/lol-champ-select/v1/sfx-notifications", ChampSelectSFXChanged);

    api.Disconnected += async (object sender, EventArgs e) => await api.ReconnectAsync();
}

As described in the earlier messages above, in your EnsureConnectionAsync(LeagueClientApi api), I also updated the Task.Delay to Task.Delay(5000).

mentorfloat commented 3 years ago

The while loop in public async Task<bool> WaitForProcess() might be your culprit, you can investigate along that line. I traced the 80004005 Exception back to that method.

bryanhitc commented 3 years ago

Hey @mentorfloat and @insaneKane, thanks for being patient! I finally was able to find some time to look into this.

Thanks for the code snippet; I was able to periodically reproduce the issue, which gave me more insight into the cause. It looks like there's an issue with the web socket library LCUSharp depends on, so I decided to change it. The old library hasn't received any updates in the past few years, so I think this is a good change to make anyway.

With that said, this does cause a breaking change where LCUSharp will now require .NET Standard 2.1 instead of 2.0. This means that regular .NET Framework will no longer be supported. Since this library doesn't exist on Nuget, I'm comfortable making this change since there likely aren't that many people depending on it, but it is important to note. This chart is relevant: https://docs.microsoft.com/en-us/dotnet/standard/net-standard

Since changing the web socket library, I haven't experienced this issue, but since I wasn't able to consistently reproduce the error with the other library, it's possible I just haven't experienced it yet. Do you mind pulling down the bryanhitc/new-websocket-library branch and testing it with your code? If everything seems to work for you as well, then I'll open a PR and merge the change.

If you have any other questions/concerns, feel free to let me know!