Popover directive with delay can cause a memory leak

[EvyatarBHA]

In Tooltip directive there's _delaySubscription member, which is unsubscribed in ngOnDestroy method. However, in Popover directive there's const _timer which isn't being handled in ngOnDestroy method. This can cause issues in case directive is being destroyed while subscription is active. In our case, it happened when we used the Popover directive to show additional row details in an ag-grid cell renderer after hovering for 500ms, but it was destroyed when the grid was scrolled.

In addition, both directives contains an old and unused member _delayTimeoutId, which better be deleted regardless.

Versions of ngx-bootstrap, Angular, and Bootstrap: ngx-bootstrap: 12.0.0 (but exists in latest code too) Angular: 17.3.0 Bootstrap: 5.3.3

Expected behavior Delay timer subscription should be unsubscribed if needed in ngOnDestroy method of Popover directive

[lexasq]

Tooltip unsubscribe triggers on close trigger, could you please share reproduction or maybe you could use this event and avoid leaks.

[EvyatarBHA]

Issue is in Popover directive. My solution for now was using Tooltip directive instead, as it doesn't contain this issue, but I assume Popover should be fixed regardless. Essentially same member behavior can be copied from Tooltip to Popover directive. Simply compare the directives' code and see the difference in regards to delayed showing. When using Popover, in case you try destroying the component with the directive while the timer subscription is alive, it will not be destroyed and popover would display despite not needed anymore.

[lexasq]

My mistake, Popover close trigger clears timer. Popover and Tooltip are not the same, if you like to provide reproduction there might be a way to show you how it should be done.

[EvyatarBHA]

The problem is with this subscription -

const _timer = timer(this.delay).subscribe(() => {
        showPopover();
        cancelDelayedTooltipShowing();
      });

If component gets destroyed before reaching delay timeout and close trigger is somehow missed. It would be complex to show our scenario, as it was a rich delayed tooltip (using Popover directive) for displaying additional data inside ag-grid cell renderer. We had triggers="mouseenter:mouseleave" and a short delay defined.

[lexasq]

Without a sample that would be hard to help you.

[EvyatarBHA]

Again, I don't need help here. For me, the solution was switching to Tooltip directive, where the bug doesn't exist. However, this is a widely used project and the solution seems to pretty straight forward - Instead of in-function const, use class member like in Tooltip directive, where same functionality is implemented slightly better.