SignalR OnDisconnected - a reliable way to handle

Question

I'm implementing a chat room. So far, so good - users can send messages from their browsers via a JS client, and I can use a C# client to do the same thing - these messages get broadcast to other users. Now, I'm trying to implement "online users".

My approach is the following:

• OnConnected - update the User in the db to be IsOnline = true
• OnDisconnected - if the User doesn't have any other connections, update the user in the db to be IsOnline = false
• I'm storing state in the DB because I have to query the db for user thumbnails anyways - this seemed like a simple alternative to working with the Dictionaries in the hub.

The problem I'm encountering is that OnDisconnected doesn't always get called for every client Id - the stale connections are preventing the "if the user doesn't have any other connections" bit from resolving to true, so the user is always "online".

One hacky solution I can think of is to always set the user to offline in the db upon OnDisconnect - but this means that if the user opens two tabs and closes one, they will be "offline". I could then re-set the user to online for every message that gets sent, but this seems like a total waste of processing cycles and still leaves a chunk of time where the user is seen as offline, when they are really online.

I believe that if there was a way to guarantee that OnDisconnected gets called for every client, this problem would go away. It seems like if I leave clients open for a long time (> 10 minutes) and then disconnect, OnDisconnected never gets called. I'll try my best to pinpoint the repro steps and keep this updated.

So - Is this a valid approach to handling online status? If so, what else can be done to ensure that OnDisconnected is firing for every connection, eventually?

This problem worries me because existing Connections will just continue to grow over time, if I'm not mistaken, eventually overflowing due to unhandled state connections.

Code:

I'm using the In-memory approach to groupings.

Sending Messages (C#):

private readonly static ConnectionMapping<string> _chatConnections =
            new ConnectionMapping<string>();
public void SendChatMessage(string key, ChatMessageViewModel message) {
            message.HtmlContent = _compiler.Transform(message.HtmlContent);
            foreach (var connectionId in _chatConnections.GetConnections(key)) {
                Clients.Client(connectionId).addChatMessage(JsonConvert.SerializeObject(message).SanitizeData());
            }
        }

State management:

    public override Task OnConnected() {
        HandleConnection();
        return base.OnConnected();
    }

    public override Task OnDisconnected() {
        HandleConnection(true);
        return base.OnDisconnected();
    }

    public override Task OnReconnected() {
        HandleConnection();
        return base.OnReconnected();
    }

    private void HandleConnection(bool shouldDisconnect = false) {
        if (Context.User == null) return;
        var username = Context.User.Identity.Name;
        var _userService = new UserService();
        var key = username;

        if (shouldDisconnect) {
                _chatConnections.Remove(key, Context.ConnectionId);
                var existingConnections = _chatConnections.GetConnections(key);
                // this is the problem - existingConnections occasionally gets to a point where there's always a connection - as if the OnDisconnected() never got called for that client
                if (!existingConnections.Any()) { // THIS is the issue - existingConnections sometimes contains connections despite there being no open tabs/clients
                    // save status serverside
                    var onlineUserDto = _userService.SetChatStatus(username, false);
                    SendOnlineUserUpdate(_baseUrl, onlineUserDto, false);
                }
        } else {
                if (!_chatConnections.GetConnections(key).Contains(Context.ConnectionId)) {
                    _chatConnections.Add(key, Context.ConnectionId);
                }
                var onlineUserDto = _userService.SetChatStatus(Context.User.Identity.Name, true);
                SendOnlineUserUpdate(_baseUrl, onlineUserDto, true);
                // broadcast to clients
        }
    }

ConnectionMapping:

public class ConnectionMapping<T> {
        private readonly Dictionary<T, HashSet<string>> _connections =
            new Dictionary<T, HashSet<string>>();

        public int Count {
            get {
                return _connections.Count;
            }
        }

        public void Add(T key, string connectionId) {
            lock (_connections) {
                HashSet<string> connections;
                if (!_connections.TryGetValue(key, out connections)) {
                    connections = new HashSet<string>();
                    _connections.Add(key, connections);
                }

                lock (connections) {
                    connections.Add(connectionId);
                }
            }
        }

        public IEnumerable<string> GetConnections(T key) {
            HashSet<string> connections;
            if (_connections.TryGetValue(key, out connections)) {
                return connections.ToList();
            }
            return Enumerable.Empty<string>();
        }

        public void Remove(T key, string connectionId) {
            lock (_connections) {
                HashSet<string> connections;
                if (!_connections.TryGetValue(key, out connections)) {
                    return;
                }

                lock (connections) {
                    connections.Remove(connectionId);

                    if (connections.Count == 0) {
                        _connections.Remove(key);
                    }
                }
            }
        }
    }

Update

Per dfowler's suggestion, an alternative approach would be to implement in-db mapping instead of in-memory, this way more metadata can be used to identify zombified connections. I'm hoping for a solution to the in-memory problem though, instead of re-architect away from a recommended approach that's already implemented.

Answer 1

Try following this sample here:

https://github.com/DamianEdwards/NDCLondon2013/tree/master/UserPresence