[CSHARP-1945] MongoServer instance leaking memory Created: 17/Mar/17  Updated: 16/Nov/21  Resolved: 23/Mar/17

Status: Closed
Project: C# Driver
Component/s: Configuration
Affects Version/s: 2.4.3
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Thomas Hjorslev Assignee: Robert Stam
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File 500calls.png    
Issue Links:
Related
related to CSHARP-1946 Edit documentation to clarify the pro... Closed

 Description   

While updating an older application to the latest driver, I needed to implement a method which took a connection string and returned a MongoDatabase instance.
I ended up with this implementation:

var url = MongoUrl.Create( connectionString );
var serverSettings = MongoServerSettings.FromUrl( url );
var s = new MongoServer( serverSettings );
var db = s.GetDatabase( url.DatabaseName );
return db;

However, after deploying the application to production, we discovered that it was slowly leaking memory. After some investigation, I found that after calling this method 500 times, a number of objects had been created, which would not be garbage collected:

MongoServer is not disposable and I cannot find any references in the documentation about how to "clean up".
I am now using this implementation, which does not seem to leak:

var url = MongoUrl.Create( connectionString );
var client = new MongoClient(url);
var s = client.GetServer();
var db = s.GetDatabase( url.DatabaseName );
return db;

However, GetServer() is deprecated, so I expect it will go away at some point.
I realize that I "should" be using IMongoDatabase instead of MongoDatabase so I could just use client.GetDatabase(...) but this is a large code base, and the API of IMongoDatabase is radically different (and worse IMHO).



 Comments   
Comment by Thomas Hjorslev [ 17/Mar/17 ]

Thank you for looking into it. It's fair that it won't be fixed if the docs describe the recommended code path.

I realize this is off-topic but it would add a lot of value if the documentation included a porting cheat sheet from 1.x to 2.x I recently upgraded this application from the 1.x line of drivers and found it surprisingly complicated.

I remember actually starting out with MongoClient.GetServer(), but dismissing it because it was deprecated and therefore using the constructor instead

Comment by Robert Stam [ 17/Mar/17 ]

I've created the related CSHARP-1946 ticket for the requested documentation change.

Comment by Robert Stam [ 17/Mar/17 ]

I think I understand the memory leak.

A legacy MongoServer instance is ultimately backed by an internal Cluster instance. If you create 500 MongoServer instances they will all share the same underlying Cluster instance.

Cluster instances are recorded in the ClusterRegistry so that they can be reused over and over.

The memory leak arises because the MongoServer instances register themselves as event handlers with the Cluster. Since the Cluster never goes away and it holds a reference to the MongoServer instances (via the event handlers) the MongoServer instances never go away either.

I don't think we will address this since it is easily avoided by using the recommended code path (i.e. calling client.GetServer).

Thanks for pointing the issue out though. It's good to have in our knowledge base.

Comment by Thomas Hjorslev [ 17/Mar/17 ]

Thank you for the explanation. At present, the documentation of the MongoServer constructor says:

Creates a new instance of MongoServer. Normally you will use one of the Create methods instead of the constructor to create instances of this class.

But since the Create methods have been removed (or made private) it's not really clear how to get an instance. May I respectfully suggest a clarification of the documentation?

We don't know yet at what point the Legacy API will be removed, but the new 2.x API was released in April 2015 so it's been quite a long time now.

Here's my vote for keeping it for many years to come

Comment by Robert Stam [ 17/Mar/17 ]

I'm not sure yet why there should be a memory leak, but I can tell you that it was not intended that an application should call the MongoServer constructor directly.

The intended way to acquire a MongoServer instance is:

var client = new MongoClient(connectionString);
var server = client.GetServer();

You are correct that at some point the deprecated GetServer method will be removed. But at that point the entire Legacy API will be removed so calling the MongoServer constructor directly won't work around the GetServer method going away.

We don't know yet at what point the Legacy API will be removed, but the new 2.x API was released in April 2015 so it's been quite a long time now.

Generated at Wed Feb 07 21:41:09 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.