Skip to content

Commit

Permalink
Never unsubscribe before receiving connected (#58)
Browse files Browse the repository at this point in the history
I think a race here explains the intermittent test failures that recently became more prominent.

I suspect it also fixes the memory leak observed by @dannyvanhoof in #49 . My theory is that if unsubscribe is called too soon, the subscription is still created after the component is unmounted and left dangling in memory. I have not done thorough testing of this though.
  • Loading branch information
Alec Larsen authored Sep 22, 2020
1 parent f52e6eb commit 439006e
Showing 1 changed file with 18 additions and 4 deletions.
22 changes: 18 additions & 4 deletions javascript/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,29 @@ import { version } from '../package.json'

export default class Component {
constructor (client, element) {
this._isShutdown = false

this.client = client
this.element = element

this._beforeConnect()

this._subscription = this.client.consumer.subscriptions.create(
const subscription = this.client.consumer.subscriptions.create(
{
channel: 'Motion::Channel',
version,
state: this.element.getAttribute(this.client.stateAttribute)
},
{
connected: () => this._connect(),
connected: () => {
if (this._isShutdown) {
subscription.unsubscribe()
return
}

this._subscription = subscription
this._connect()
},
rejected: () => this._connectFailed(),
disconnected: () => this._disconnect(),
received: newState => this._render(newState)
Expand Down Expand Up @@ -48,8 +58,12 @@ export default class Component {
}

shutdown () {
this._subscription.unsubscribe()
delete this._subscription
this._isShutdown = true

if (this._subscription) {
this._subscription.unsubscribe()
delete this._subscription
}

this._disconnect()
}
Expand Down

0 comments on commit 439006e

Please sign in to comment.