[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: |
|
||||||||
| Issue Links: |
|
||||||||
| 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.
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:
However, GetServer() is deprecated, so I expect it will go away at some point. |
| 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 | ||
| 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:
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?
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:
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. |