Skip to content
This repository has been archived by the owner. It is now read-only.

Calling Clients.All.SendAsync from a secondary thread always throws ObjectDisposedException #2424

Closed
ThadHouse opened this issue Jun 3, 2018 · 7 comments

Comments

@ThadHouse
Copy link

@ThadHouse ThadHouse commented Jun 3, 2018

I am working on adding SignalR to a project of mine. I have a callback thread in my library, and I want to send that callback over SignalR whenever I receive one. However, I always seem to be getting ObjectDisposedException while sending from the callback thread, even if I can guarantee the client is still connected. Do I need to post the SendAsync to the Hub context, and if so how, because I can't find the synchronization context for the hub.

Using 1.0.0 of both client and C# server.

Client Code (Console App)

        static async Task Main(string[] args)
        {

            HubConnection connection = new HubConnectionBuilder().
                WithUrl("https://localhost:44311/nthub").Build();

            connection.On<int>("HelloWorld", (b) =>
            {
                ; // Never gets hit
            });

            await connection.StartAsync();

            while (true)
            {

                await Task.Delay(1000);
            }
        }

Server Hub Code (ASP.NET Core 2.1)

public class NTHub : Hub
    {
        private volatile bool active = true;
        Thread t;

        public override Task OnConnectedAsync()
        {
           // This will normally enable my callback in my library
           // But minimal test is just a thread. 
            t = new Thread(() =>
            {
                while (active)
                {
                    Thread.Sleep(1000);
                    this.Clients.All.SendAsync("HelloWorld", 55).Wait(); // Throws ObjectDisposedException
                }
            });
            t.Start();
            return base.OnConnectedAsync();
        }

        public override Task OnDisconnectedAsync(Exception exception)
        {
            // This would normally unregister my callbacks.
            active = false;
            t.Join();
            return base.OnDisconnectedAsync(exception);
        }
    }

I understand the thread safety issues of the server side, but thats just the minimal example. My actual code handles multiple clients properly.

@davidfowl

This comment has been minimized.

Copy link
Member

@davidfowl davidfowl commented Jun 3, 2018

By design, hubs are short lived. You can’t capture state from the hub to do long running things. It is disposed after every invocation.

PS: that code will not scale at all. It’s spawning a new thread per connection which is pretty bad. What are you trying to do?

@ThadHouse

This comment has been minimized.

Copy link
Author

@ThadHouse ThadHouse commented Jun 3, 2018

That was just a minimal example. Theres only 1 thread running in the background handling a list of callbacks. When a connection gets added, it should just register a callback. There is definitely not a thread per connection, that was just a way to show the issue easily. Basically I just want to notify my webpage every time one of those callbacks occur. I thought that was one of the advantages of SignalR, that it would actually keep state. Its not another webpage sending data, but actually something from the server.

@ThadHouse

This comment has been minimized.

Copy link
Author

@ThadHouse ThadHouse commented Jun 3, 2018

Based on some stackoverflow answers, I figured out how to do broadcast with DI and an IHubContext. That should be something in the docs on how to do broadcast.

@davidfowl

This comment has been minimized.

Copy link
Member

@davidfowl davidfowl commented Jun 3, 2018

Oh there are definitely ways to do it, just not like what you showed. It looks like you’re just broadcasting from some external event? What do you need to put code in the hub? Can’t yju just inject the IHubContext into your background processor (which it seems like you only need one callback, instead of one per connection).

@ThadHouse

This comment has been minimized.

Copy link
Author

@ThadHouse ThadHouse commented Jun 3, 2018

There were no docs on how to do it, so since the hub had the context it seemed like that would work. Just something added to the docs on how to do broadcast would be nice.

@davidfowl

This comment has been minimized.

Copy link
Member

@davidfowl davidfowl commented Jun 3, 2018

The docs are a bit light but we expect them to get beefier this week with most of the common examples

@anurse

This comment has been minimized.

Copy link
Member

@anurse anurse commented Jun 8, 2018

aspnet/AspNetCore.Docs#5493 is tracking the work to add docs for HubContext. I'm closing this as the docs should help resolve the issue.

@anurse anurse closed this Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.