31

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.

9
  • Have you tried with different configurations for the DisconnectTimeout, KeepAlive and Heatbeat intervals? github.com/SignalR/SignalR/wiki/Configuring-SignalR Jan 11, 2014 at 21:45
  • @thepirat000 - yeah I have... this happens on the scale of hours/days too - connections are staying alive in memory indefinitely.
    – SB2055
    Jan 11, 2014 at 21:48
  • Don't use database for online/offline detection.. that should be part of the server just do connections.Count or connections.Length to get the count. Because if your server crashes you'll have many online users when you just start it up..
    – SSpoke
    Jan 12, 2014 at 3:55
  • 1
    Try turning tracing on for the transports github.com/SignalR/SignalR/blob/master/samples/…. It'll show details for specific connection ids as well as lifetime events that get raised or not raised in your case.
    – davidfowl
    Jan 12, 2014 at 6:03
  • 1
    You can't count on getting a nice message from the client when they leave; networks can just vanish (and actually do so all the time). Use your heartbeat code to handle disconnects, except when you're lucky enough to get an explicit session-end notification. Don't count on being lucky. Jan 12, 2014 at 11:50

1 Answer 1

21

Try following this sample here:

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

13
  • I was crossing my fingers in hopes that you'd see this. I'm looking now - what parts exactly? I'm using the in-memory approach to mapping users <> connections. This seems to use a different approach.
    – SB2055
    Jan 12, 2014 at 3:34
  • In case a full implementation details would help - here's everything: codereview.stackexchange.com/questions/39073/…
    – SB2055
    Jan 12, 2014 at 3:42
  • 2
    The in memory approach has some obvious weaknesses: - If the server restarts you lose all state - If you have multiple servers it'll be inaccurate (as each server will have a different view of who is "online") If the 2 above problems aren't a concern then it should be fine as long as you understand when OnReconnected and OnDisconnected would happen. OnReconnected can fire for e.g. after an app pool/app domain recycle where you lose all of your state and some clients will reconnect and others will disconnect and other will connect.
    – davidfowl
    Jan 12, 2014 at 6:01
  • 1
    I use azure cache... i think you will still run into problems. I solve this by pinging from the clients and setting a threshold of inactivity. Surefire way although not as real time. May 13, 2014 at 15:42
  • 1
    Imagine the server crashes. What happens then?
    – davidfowl
    Jul 6, 2021 at 6:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.