QuerySession.ForTenant disposal

[fawques]

We have a class with an IQuerySession injected in the constructor, and some methods that scope that session to a specific tenant with .ForTenant(tenant).
We call some of the methods more than once, in the same scope.
As ITenantQueryOperations implements IDisposable, we were calling it with using. However, when doing that, the tenant session returned is the same instance that was already disposed.

Looking into the code, I see the tenant session uses the same connection as the parent session, should we avoid disposing the sessions returned in ForTenant?

[fawques]

I attach some sample code with what I thought was correct code:

public class Service
{
	private readonly IQuerySession session;
	public Service(IQuerySession session)
	{
		this.session = session;
	}
	
	public void Method1(string tenant, Guid id)
	{
		using var tenantSession = session.ForTenant(tenant);	
		var agg = tenantSession.Load<Aggregate>(id);
		// ...
	}
	
	
	public void Method2(string tenant, Guid id)
	{
		using var tenantSession = session.ForTenant(tenant);	
		var agg = tenantSession.Load<Aggregate>(id);
		// ...
	}
}

called with

service.Method1("tenant");
service.Method2("tenant");

throws ObjectDisposedException.
The IQuerySession in the service is not disposed, but the returned tenant-scoped session is.

[elexisvenator]

The tenanted query session is internally cached and reused, so I think you should avoid disposing of the tenantSession, and instead rely on your DI disposing the scoped IQuerySession.

If you want to keep disposing of sessions when you are done with them (which feels cleaner imo), what you can do is DI the IDocumentStore instead and call documentStore.QuerySession(tenant) to create a full query session for the specific tenant.

[jeremydmiller]

Ugh, don't call Dispose() on ITenantOperations. That' a massive oversight on my part by just having that implement IQuerySession. By disposing the ITenantOperations you're more or less disposing the parent. Unintended consequences.

I'm pushing a quick fix to 6.0 right now that makes the dispose on ITenantOperations do nothing so you don't actually cause any accidental harm.