From 749df70d5c25e02894eed98a5f38912bf1230902 Mon Sep 17 00:00:00 2001 From: Thomas Nordquist Date: Wed, 24 Apr 2019 23:40:28 +0200 Subject: [PATCH] Fix memory leaks --- app/src/actions/Connection.ts | 3 +++ app/src/actions/Tree.ts | 24 +++++++++++++++-------- app/src/components/Sidebar/Sidebar.tsx | 5 +---- app/src/components/Tree/Tree.tsx | 4 ++-- app/src/components/Tree/TreeNode.tsx | 8 ++++---- app/src/model/TopicViewModel.ts | 2 +- backend/src/DataSource/DataSourceState.ts | 2 +- backend/src/Model/Tree.ts | 9 ++++++++- backend/src/Model/TreeNode.ts | 21 +++++++++++++++++--- backend/src/index.ts | 2 +- events/EventDispatcher.ts | 11 +++-------- 11 files changed, 58 insertions(+), 33 deletions(-) diff --git a/app/src/actions/Connection.ts b/app/src/actions/Connection.ts index b4ff095..a245b31 100644 --- a/app/src/actions/Connection.ts +++ b/app/src/actions/Connection.ts @@ -78,6 +78,8 @@ export const disconnect = () => (dispatch: Dispatch, getState: () => AppSta } tree && tree.stopUpdating() + tree && tree.destroy() + // Clear topic filter dispatch({ topicFilter: '', @@ -88,4 +90,5 @@ export const disconnect = () => (dispatch: Dispatch, getState: () => AppSta dispatch({ type: ActionTypes.CONNECTION_SET_DISCONNECTED, }) + dispatch(showTree(undefined)) } diff --git a/app/src/actions/Tree.ts b/app/src/actions/Tree.ts index 7f4ac67..687f474 100644 --- a/app/src/actions/Tree.ts +++ b/app/src/actions/Tree.ts @@ -53,20 +53,28 @@ const debouncedSelectTopic = debounce((topic: q.TreeNode, dispat } }, 70) -export const resetStore = () => (dispatch: Dispatch): AnyAction => { +function destroyUnreferencedTree(state: AppState) { + const visibleTree = state.tree.get('tree') + const connectionTree = state.connection.tree + + // Stop updates of old tree + if (visibleTree && visibleTree !== connectionTree) { + console.warn('destroy') + visibleTree.stopUpdating() + visibleTree.destroy() + } +} + +export const resetStore = () => (dispatch: Dispatch, getState: () => AppState): AnyAction => { + destroyUnreferencedTree(getState()) + return dispatch({ type: ActionTypes.TREE_RESET_STORE, }) } export const showTree = (tree: q.Tree | undefined) => (dispatch: Dispatch, getState: () => AppState): AnyAction => { - const visibleTree = getState().tree.get('tree') - const connectionTree = getState().connection.tree - - // Stop updates of old tree - if (visibleTree !== connectionTree && visibleTree) { - visibleTree.stopUpdating() - } + destroyUnreferencedTree(getState()) return dispatch({ tree, diff --git a/app/src/components/Sidebar/Sidebar.tsx b/app/src/components/Sidebar/Sidebar.tsx index f078d3f..ade7a1f 100644 --- a/app/src/components/Sidebar/Sidebar.tsx +++ b/app/src/components/Sidebar/Sidebar.tsx @@ -35,7 +35,6 @@ interface Props { } interface State { - node: q.TreeNode compareMessage?: q.Message valueRenderWidth: number } @@ -49,8 +48,7 @@ class Sidebar extends React.Component { constructor(props: any) { super(props) - console.error('Find and fix me #state') - this.state = { node: new q.Tree(), valueRenderWidth: 300 } + this.state = { valueRenderWidth: 300 } } private registerUpdateListener(node: q.TreeNode) { @@ -156,7 +154,6 @@ class Sidebar extends React.Component { public componentWillReceiveProps(nextProps: Props) { this.props.node && this.removeUpdateListener(this.props.node) nextProps.node && this.registerUpdateListener(nextProps.node) - this.props.node && this.setState({ node: this.props.node }) if (this.props.node !== nextProps.node) { this.setState({ compareMessage: undefined }) diff --git a/app/src/components/Tree/Tree.tsx b/app/src/components/Tree/Tree.tsx index 191b99d..509b726 100644 --- a/app/src/components/Tree/Tree.tsx +++ b/app/src/components/Tree/Tree.tsx @@ -30,7 +30,7 @@ interface State { lastUpdate: number } -class Tree extends React.PureComponent { +class TreeComponent extends React.PureComponent { private updateTimer?: any private perf: number = 0 private renderTime = 0 @@ -137,4 +137,4 @@ const mapDispatchToProps = (dispatch: any) => { } } -export default connect(mapStateToProps, mapDispatchToProps)(Tree) +export default connect(mapStateToProps, mapDispatchToProps)(TreeComponent) diff --git a/app/src/components/Tree/TreeNode.tsx b/app/src/components/Tree/TreeNode.tsx index 28a1d36..30a1cf7 100644 --- a/app/src/components/Tree/TreeNode.tsx +++ b/app/src/components/Tree/TreeNode.tsx @@ -70,7 +70,7 @@ interface State { selected: boolean } -class TreeNode extends React.Component { +class TreeNodeComponent extends React.Component { private animationDirty: boolean = false private cssAnimationWasSetAt?: number @@ -97,8 +97,8 @@ class TreeNode extends React.Component { treeNode.viewModel.change.subscribe(this.viewStateHasChanged) } - private viewStateHasChanged = (msg: void, viewModel: TopicViewModel) => { - this.setState({ selected: viewModel.isSelected() }) + private viewStateHasChanged = (msg: void) => { + this.setState({ selected: this.props.treeNode.viewModel!.isSelected() }) } private removeSubscriber(treeNode: q.TreeNode) { @@ -243,4 +243,4 @@ class TreeNode extends React.Component { } } -export default withStyles(styles, { withTheme: true })(TreeNode) +export default withStyles(styles, { withTheme: true })(TreeNodeComponent) diff --git a/app/src/model/TopicViewModel.ts b/app/src/model/TopicViewModel.ts index c245aab..d13247a 100644 --- a/app/src/model/TopicViewModel.ts +++ b/app/src/model/TopicViewModel.ts @@ -2,7 +2,7 @@ import { EventDispatcher } from '../../../events' export class TopicViewModel { private selected: boolean - public change = new EventDispatcher(this) + public change = new EventDispatcher() public constructor() { this.selected = false diff --git a/backend/src/DataSource/DataSourceState.ts b/backend/src/DataSource/DataSourceState.ts index 861304c..abffb36 100644 --- a/backend/src/DataSource/DataSourceState.ts +++ b/backend/src/DataSource/DataSourceState.ts @@ -7,7 +7,7 @@ export interface DataSourceState { } export class DataSourceStateMachine { - public onUpdate = new EventDispatcher(this) + public onUpdate = new EventDispatcher() private state: DataSourceState = { error: undefined, connected: false, diff --git a/backend/src/Model/Tree.ts b/backend/src/Model/Tree.ts index fbc0b4f..d3c7311 100644 --- a/backend/src/Model/Tree.ts +++ b/backend/src/Model/Tree.ts @@ -16,7 +16,7 @@ export class Tree extends TreeNode { public isTree = true private cachedHash = `${Math.random()}` private unmergedMessages: ChangeBuffer = new ChangeBuffer() - public didReceive = new EventDispatcher>(this) + public didReceive = new EventDispatcher>() constructor() { super(undefined, undefined) @@ -27,6 +27,13 @@ export class Tree extends TreeNode { this.didReceive.dispatch() } + public destroy() { + super.destroy() + this.updateSource && this.updateSource.unsubscribe(this.subscriptionEvent, this.handleNewData) + this.updateSource = undefined + this.didReceive.removeAllListeners() + } + public updateWithConnection(emitter: EventBusInterface, connectionId: string, nodeFilter?: (node: TreeNode) => boolean) { this.updateSource = emitter this.connectionId = connectionId diff --git a/backend/src/Model/TreeNode.ts b/backend/src/Model/TreeNode.ts index b00023b..bd1c8ba 100644 --- a/backend/src/Model/TreeNode.ts +++ b/backend/src/Model/TreeNode.ts @@ -12,9 +12,9 @@ export class TreeNode { public collapsed = false public messages: number = 0 public lastUpdate: number = Date.now() - public onMerge = new EventDispatcher>(this) - public onEdgesChange = new EventDispatcher>(this) - public onMessage = new EventDispatcher>(this) + public onMerge = new EventDispatcher>() + public onEdgesChange = new EventDispatcher>() + public onMessage = new EventDispatcher>() public isTree = false private cachedPath?: string @@ -99,6 +99,21 @@ export class TreeNode { } } + public destroy() { + for (const edge of this.edgeArray) { + edge.target.destroy() + } + this.edgeArray = [] + this.edges = {} + this.cachedChildTopics = [] + this.sourceEdge = undefined + this.onMerge.removeAllListeners() + this.onEdgesChange.removeAllListeners() + this.onMessage.removeAllListeners() + this.messageHistory = new RingBuffer(1, 1) + this.message = undefined + } + public unconnectedClone() { const node = new TreeNode() node.message = this.message diff --git a/backend/src/index.ts b/backend/src/index.ts index b0d8156..39aea1e 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -78,7 +78,7 @@ export class ConnectionManager { } class UpdateNotifier { - public onCheckUpdateRequest = new EventDispatcher(this) + public onCheckUpdateRequest = new EventDispatcher() constructor() { backendEvents.subscribe(checkForUpdates, () => { this.onCheckUpdateRequest.dispatch() diff --git a/events/EventDispatcher.ts b/events/EventDispatcher.ts index 6e1097a..2cabbf3 100644 --- a/events/EventDispatcher.ts +++ b/events/EventDispatcher.ts @@ -7,20 +7,15 @@ interface CallbackStore { export class EventDispatcher { private emitter = new EventEmitter() - private dispatcher: Dispatcher private callbacks: Array = [] - constructor(dispatcher: Dispatcher) { - this.dispatcher = dispatcher - } - public dispatch(msg: Message) { this.emitter.emit('event', msg) } - public subscribe(callback: (msg: Message, dispatcher: Dispatcher) => void) { + public subscribe(callback: (msg: Message) => void) { const wrappedCallback = (msg: Message) => { - callback(msg, this.dispatcher) + callback(msg) } this.emitter.on('event', wrappedCallback) @@ -30,7 +25,7 @@ export class EventDispatcher { }) } - public unsubscribe(callback: (msg: Message, dispatcher: Dispatcher) => void) { + public unsubscribe(callback: (msg: Message) => void) { const item = this.callbacks.find(store => store.callback === callback) if (!item) { return