Datepicker Memory Leak
Opened this issue · 1 comments
Building off #2268, there is a problem when destroying the datepicker where it doesn’t completely get removed from memory. This pull request, 817ce38, removes the instance reference but the actual UI reference is tied to
jquery-ui/ui/widgets/datepicker.js
Line 163 in 54f96ee
When the widget is destroyed, the datepicker UI element remains in memory preventing it from closing. #2268 patches this issue by hiding the component, however dpDiv
is still in memory. One solution could be to remove dpDiv from the instance and then set both _curInst = null
and this.dpDiv = null
.
5e4f1b8
However, many unit tests will break because they rely on dpDiv to still exist in memory, requiring the datepicker to be recreated for each test. There is also a problem that many developers rely on hiding the datepicker before destroying it $.datepicker("hide").destroy("destroy")
which will fail because dpDiv
will be null.
Thanks for the report.
PR #1362 fixing Trac issue #10668 and PR #1852 fixing Trac issue #15270 removed references to a Datepicker instance, which is way more than just the div representing the datepicker UI. Also, $.datepicker.dpDiv
is created only once - and even if you load multiple versions of jQuery & jQuery UI on a page, the div is only attached to the document once. Thus, this issue is not fully comparable to the previous ones.
I think we'll just need to accept this dpDiv
instance as an inherent part of Datepicker and its memory usage. There's no memory leak per se as creating multiple datepickers & destroying them will not build up memory from past destroyed date pickers. It's just maybe taking up a bit more memory that it could - but it's not even clear to me it's a bad thing as not having to recreate the datepicker container has its advantages as well.
One thing that could be considered is calling this.dpDiv.empty()
when destroy
is called on a date picker which was the most recently shown (or is still shown) using this global dpDiv
, since we already empty it on showing here:
jquery-ui/ui/widgets/datepicker.js
Lines 831 to 832 in 54f96ee
so the previous contents would not be used anyway. We need to be careful here, though, to not delete contents meant for a different datepicker instance, especially when multiple jQuery UI versions are running on the same page.
I'd be willing to review a PR, but it might not be a trivial thing to do right.
(Since the issue is already in 1.12, given limited team resources it's not likely to be fixed by the UI team; see the project status at https://blog.jqueryui.com/2021/10/jquery-maintainers-update-and-transition-jquery-ui-as-part-of-overall-modernization-efforts/.)