Halt execution until server/client is ready after calling start

Question

Halt execution until server/client is ready after calling start

Closed this issue 4 years ago · 24 comments

Is there a way to block execution until the server is actually ready? Atm I'm doing a while loop that checks the IsListening variable which seems wasteful. Same with the client, except checking if the client IsConnected.

I would love to be able to do something like:

await WebsocketServer.Start()
await WebsocketClient.Start()

Answer 1 · 2021-02-13T04:22:19.000Z

We could add a StartAsync method to both, would that work? In the server it would return the AcceptConnections task, and in the client, it would return the ConnectAsync task; both could be awaited.

Answer 2 · 2021-02-15T05:50:50.000Z

Hi @Alig96 I just pushed v2.2.1 to NuGet which adds a StartAsync to both client and server. Mind giving it a try and letting me know if this is what you need?

NuGet: https://www.nuget.org/packages/WatsonWebsocket/2.2.1
Commit: f57cb56

Answer 3 · 2021-02-15T07:51:46.000Z

Awesome, thanks for the quick update, I have got some good news and bad news;

I was able to replace my wait loop for the client and works great
I was not able to replace my wait loop for the server as the await from await server.StartAsync() never completes. I upped my test time out from 2 seconds to 30 and it still doesn't complete the await

Any suggestions?

Answer 4 · 2021-02-15T15:11:42.000Z

Which OS and framework are you using? I just tested with the Test.Server and Test.Client project and it worked. Will need some help reproducing.

Answer 5 · 2021-02-15T15:23:15.000Z

OS: MacOs Catalina (10.15.7)
Framework: net5.0

Here's a reproduction sample (Requires NUnit)

using System;
using System.Threading.Tasks;
using NUnit.Framework;
using WatsonWebsocket;

namespace WatsonServerHoldDemo
{
    public class Tests
    {
        [Test]
        [Timeout(5000)]
        public async Task Test1()
        {
            var server = new WatsonWsServer();
            await server.StartAsync();
            Console.WriteLine("online :)");
        }
    }
}

"Online :)" is never hit :(

I can also attempt to try this on Ubuntu later tonight if you beleive this is an o/s issue.

Answer 6 · 2021-02-15T15:51:54.000Z

Hi @Alig96 I think it might be. I'll try on my Mac (Big Sur) in a moment.

Answer 7 · 2021-02-15T16:03:12.000Z

Instead of awaiting it, could you assign it to a Task? i.e. Task t = server.StartAsync()?

The call to StartAsync is running the task, then returning that task back to you, so you don't need to await it.

            _AcceptConnectionsTask = Task.Run(() => AcceptConnections(), _Token);
            return _AcceptConnectionsTask;

FWIW, I just reproduced what you're seeing in Catalina, so it's not an OS issue.

This is using Task task = server.StartAsync():

Answer 8 · 2021-02-15T19:10:28.000Z

I could yes, but then wouldn't I be back to square one as there wouldn't be a difference between server.Start() & server.StartAsync()? Is it not possible to fix the await issue?

Answer 9 · 2021-02-15T19:15:21.000Z

You're right, this implementation still requires you to loop over IsListening. Sorry about that, it didn't fix the root issue.

I'll move the call to start the listener into both Start and StartAsync. Give me a moment.

Answer 10 · 2021-02-15T19:20:47.000Z

This one has the listener starting inside of Start and StartAsync. You should see IsListening == true now after both. Please let me know if this works:

NuGet: https://www.nuget.org/packages/WatsonWebsocket/2.2.1.1
Commit: e502aa6

Answer 11 · 2021-02-15T20:15:06.000Z

Updated and removed my while loop so thats great. Thanks for your work. However I still have a visual/semantics issue with not being able to await the async start;

Using the regular start within my (already async) function i get this back from Rider:

Which when I change to use the async overload

It pushes me to use the async tag (which when I add it, even in the new build) still fails to return and dead locks :(

Answer 12 · 2021-02-15T20:54:59.000Z

Bah, you're right. I was thinking the main usage would be Task t = server.StartAsync(), but a better approach is to await server.StartAsync() with an optional CancellationToken. One moment, updating.

Answer 13 · 2021-02-15T20:56:30.000Z

This will (hopefully) do the trick:

NuGet: https://www.nuget.org/packages/WatsonWebsocket/2.2.1.2
Commit: 33a333c

Please let me know!

Answer 14 · 2021-02-15T21:25:16.000Z

Still hangs unfortunatly :(

using System;
using System.Threading.Tasks;
using NUnit.Framework;
using WatsonWebsocket;

namespace WatsonServerHoldDemo
{
    public class Tests
    {
        [Test]
        [Timeout(5000)]
        public async Task Test1()
        {
            var server = new WatsonWsServer
            {
                Logger = Console.WriteLine
            };

            await server.StartAsync();

            var client = new WatsonWsClient("localhost", 9000, false)
            {
                Logger = Console.WriteLine
            };

            await client.StartAsync();

            Console.WriteLine(client.Connected);
        }
    }
}

Answer 15 · 2021-02-15T22:30:14.000Z

Looking into it now

Answer 16 · 2021-02-15T22:33:43.000Z

Let me try on Mac. It's working on Windows 10 with .NET 5:

Code:

Answer 17 · 2021-02-15T22:43:12.000Z

I'm not seeing a hang on .NET5 on Mac either:

The right terminal window is the client. Notice it shows Client started: true. Do you see this issue when using the Test.Client project?

Answer 18 · 2021-02-16T13:21:11.000Z

I have recreated the issue in your test server project:

Line #147

        static async void StartServer()
        {                         
            // _Server.Start();
            Console.WriteLine("Starting...");
            await _Server.StartAsync();
            Console.WriteLine("Finished!");
        }

Log

Server IP: [localhost] 
Server port: [9000] 
Use SSL: [y/N]? n
Command [? for help]: start
Starting...
[WatsonWsServer] starting http://localhost:9000/
Command [? for help]:

Starting is called and the server does indeed start up, but notice Finished! is never called in the log.

Answer 19 · 2021-02-16T15:48:36.000Z

Thanks @Alig96 - just reproduced, debugging now. Will report back shortly.

Answer 20 · 2021-02-16T16:12:17.000Z

Hi @Alig96 here's the challenge I'm running into while trying to devise an outcome that will allow you to await server.StartAsync().

StartAsync calls AcceptConnections which is a while loop. I wouldn't normally use a while loop in this case, but the call to listener.GetContextAsync() is blocking, so I don't worry about CPU usage with it.

If I just return the Task for AcceptConnections, and it is awaited, it will block because the while loop is still running.

If I have AcceptConnections kick off an unawaited Task containing the while loop, it won't block, but then you won't have the actual Task.

Would love your input on how you'd like me to proceed on this one. I'm of the opinion that having the Task itself isn't super valuable, since you can pass in your own CancellationToken on StartAsync which will give you control over canceling it.

Thoughts?

Answer 21 · 2021-02-16T17:06:56.000Z

The reason I'm wanting to be able to await the server starting is so that I can guarantee that everything after that line the server is online and ready to start accepting connections and sending data. The issue I had originally was that in the example in this since its a unit test and runs fast, after the calling server.Start() the very next line would be client.Connect() which would cause the client to fail connecting because the server is not ready yet because it takes a second or two to be ready. In a real life scenario, a client would probably never connect that fast but from a testing stance this is an issue.

You’re right that I’m not actually needing the actual task of AcceptConnections back from the Start command. The internal task to me is an implementation detail and as a user of the library, I doubt I will ever require access to the actual AcceptConncections task. I also believe that I (as a user) should not have control over the CancellationToken. Seems like its leaking too much control like that, since there’s a Stop() function, shouldn’t that be enough? Since the appeal of the library is be easy to get kickstarted. The ideal API in my eyes would be:

await server.Start()
// guaranteed server is ready
await server.Stop()
// guaranteed server is completely stopped

await client.Start()
// guaranteed client is connected/ready (if not perhaps an exception if it can’t start/connect)
await client.Stop()
// guaranteed client is completely stopped

Since those will all be asynchronous by default, the user can choose if they want it to run without blocking (by not adding the await tag) or blocking by adding the await tag which covers both the scenarios.

As a quick fix for me that I would be happy with, is just return a task that is ~1 second delay. However, long term for this project though, this would be a design flaw since the server could be ready way before that (which it usually is since it is a fast library, have not actually profiled it but can tell) and I would consider seeing if there’s a way to make this better.

Answer 22 · 2021-02-16T17:10:45.000Z

This is excellent information. I agree that the Task itself isn't really useful. The primary motivation for allowing the user to influence the CancellationToken (it's an optional param) is in case they want to synchronize cancellation with other aspects of their application.

I'm going to test out a change wherein you can await StartAsync and the returned Task isn't tied to the AcceptConnections loop. Reporting back soon.

Answer 23 · 2021-02-16T17:16:07.000Z

This should do it. I'm going to do some more research into returning the actual Task without blocking should it be awaited.

NuGet: https://www.nuget.org/packages/WatsonWebsocket/2.2.1.3
Commit: f5756d8

Please close if this works for you, or let me know if any issues remain!

Ex:

await _Server.StartAsync();
Console.WriteLine("Server is listening: " + _Server.IsListening);

Answer 24 · 2021-02-16T17:33:30.000Z

Fantastic, that's done the trick. Thanks alot for your help & quick responses and deploys.

I notice that you did end up going the delay route. I hope you can refactor to return the actual task without blocking since that will make the application better in the long run.