NHibernate.ISession still open when using signalr

[MaxenceMouchard]

I integrated SignalR into my app some time after using your ISession injection implementation with Identity.

The problem is that signalr is connected with identity and ISession's injected in the identity implementation stay open.

So each Hub from SignalR Keep in Context identity and injected sessions are never kill.

So one session stay open for each Hub.

I was thinking to inject ISessionFactory in identity and open the sessions in the implemented methods for kills Sessions at each end of action but I do not know if this method is suitable, Knowing that the implementation of Users in UserStore is IQueryable so this would not be very safe to do something like:

private readonly ISessionFactory _sessionFactory;

public override IQueryable<User> Users
{
    get
    {
        using (ISession session = _sessionFactory.OpenSession())
        {
            return session.Query<User>();
        }
    }
}

public UserStore(ISessionFactory sessionFactory, IdentityErrorDescriber errorDescriber) : base(errorDescriber)
{
    _sessionFactory = sessionFactory;
}

what do you think about this case ?

Thank for your times.

[gliljas]

Returning an IQueryable from within a "using session" doesn't work. What is the broader use case?

[MaxenceMouchard]

It works, but as I said, the fact that because it's queryable, my example is not safe and shouldn't be done, I'm aware of that, but I don't see any other options.

The broader use case is just an classique of SignalR.
When a hub is connected, it's look like the scopped Session from identity is never kill when an instance of the hub is created and when the Context.Identity is attached on the Hub.

Would you like an example git repo if I'm not clear ?

[gliljas]

I don't see how it could work. The session is closed when the IQueryable is returned.

[MaxenceMouchard]

Don't get stuck on the Users property, it's not used in my implementation, it's just that Identity requires its implemation. It only works because Users is never called, but my other methods are implemented like this :

public override async Task<IdentityResult> CreateAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
    cancellationToken.ThrowIfCancellationRequested();
    ThrowIfDisposed();
    if (user == null) {
        throw new ArgumentNullException(nameof(user));
    }
    using(ISession session = _sessionFactory.OpenSession()) {
        await session.SaveAsync(user, cancellationToken);
        await FlushChangesAsync(session, cancellationToken);
    }
    return IdentityResult.Success;
}

public override async Task<IdentityResult> UpdateAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
    cancellationToken.ThrowIfCancellationRequested();
    ThrowIfDisposed();
    if (user == null) {
        throw new ArgumentNullException(nameof(user));
    }
    using(ISession session = _sessionFactory.OpenSession()) {
        var exists = await session.Query<User> ().AnyAsync(
            u => u.Id.Equals(user.Id),
            cancellationToken
        );
        if (!exists) {
            return IdentityResult.Failed(
                new IdentityError {
                    Code = "UserNotExist",
                        Description = $ "User with id {user.Id} does not exists!"
                }
            );
        }
        user.ConcurrencyStamp = Guid.NewGuid().ToString("N");


        await session.MergeAsync(user, cancellationToken);
        await FlushChangesAsync(session, cancellationToken);
        return IdentityResult.Success;
    }

}

public override async Task<IdentityResult> DeleteAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
    cancellationToken.ThrowIfCancellationRequested();
    ThrowIfDisposed();
    if (user == null) {
        throw new ArgumentNullException(nameof(user));
    }
    using(ISession session = _sessionFactory.OpenSession()) {

        await session.DeleteAsync(user, cancellationToken);
        await FlushChangesAsync(session, cancellationToken);
        return IdentityResult.Success;
    }
}

public override async Task<User?> FindByIdAsync(string userId, CancellationToken cancellationToken = new CancellationToken()) {
    cancellationToken.ThrowIfCancellationRequested();
    ThrowIfDisposed();
    var id = ConvertIdFromString(userId);

    using(ISession session = _sessionFactory.OpenSession()) {
        var user = await session.GetAsync<User> (id, cancellationToken);
        return user;
    }
}

[gliljas]

That looks like a perfectly viable solution

[MaxenceMouchard]

It should work except if try to play with a user getting by FindByIdAsync function for example ...
So I agree it's viable but not really rustworthy/safe.
Have you ever had to deal with this issue with SignalR or do you have any advice or other more conveniant approach ?

[beginor]

If you are using SignalR Hubs to send message, please follow the rules of hubs https://learn.microsoft.com/en-us/aspnet/core/signalr/hubs?view=aspnetcore-7.0#create-and-use-hubs

Please Note

Hubs are transient:

• Don't store state in a property of the hub class. Each hub method call is executed on a new hub instance.
• Don't instantiate a hub directly via dependency injection. To send messages to a client from elsewhere in your application use an IHubContext.
• Use await when calling asynchronous methods that depend on the hub staying alive. For example, a method such as Clients.All.SendAsync(...) can fail if it's called without await and the hub method completes before SendAsync finishes.

I think the safest useage should be like this:

public class MessageHub : Hub {

    public async Task SendMessage(string user, string message) {
        var factory = this.Context.GetHttpContext().RequestServices.GetService<ISessionFactory>();
        using var session = factory.OpenSession();
        var users = await session.Query<Users>().ToListAsync();
        await Clients.All.SendAsync("ReceiveMessage", user, users);
    }

}

Or try inject IServiceProvider in to you hub, like this:

public class MessageHub : Hub {

    private readonly IServiceProvider serviceProvider;

    public MessageHub(IServiceProvider serviceProvider) {
        this.serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
    }

    public async Task SendMessage(string user, string message) {
        var sessionFactory = serviceProvider.GetService<ISessionFactory>();
        using var session = sessionFactory.OpenSession();
        await Clients.All.SendAsync("ReceiveMessage", user, message);
    }

}

Not fully tested, just a suggestion;

[MaxenceMouchard]

Thank you for your advice but i don't use any injection or session inside my hubs. Hubs are clean as you defined the transiant.
My injection comes from UserStore which implements Identity and automatically binds with Signalr Hubs.

The problem is just that: Hub is automatically connected with identity and the session injected into the UserStore constructor stay open for the entire life of the application. So Each Hubs results in an additional open session.

[beginor]

By default UserStore and RoleStore are registered as Scoped (create instances per request), which is suitable for web api and mvc.

If this is not suitable for SignalR, you can register them with another life time. And this is easy, just dont use the default extension method AddHibernateStores , register them with another lifetime which are suiteable for SignalR.

PS: Any PR is welcome!