[tree]: fix infinity recursion when tree root is change during ongoin…

…g refresh

The navigator model root gets initialized too eagerly before the workspace service is ready to provide roots. And then the second time by an event then the workspace service is initialized. It causes 2 parallel refreshes and with bad timing on workspaces with multiple roots can lead to infinity loop and eventually max call stack error.

This resolves the issue generically for trees by allowing parallel refreshes without resetting a root.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>

packages/core/src/browser/tree/test/tree-test-container.ts

@@ -0,0 +1,45 @@
+/********************************************************************************
+ * Copyright (C) 2019 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { TreeImpl, Tree } from '../tree';
+import { TreeModel, TreeModelImpl } from '../tree-model';
+import { Container } from 'inversify';
+import { TreeSelectionServiceImpl } from '../tree-selection-impl';
+import { TreeSelectionService } from '../tree-selection';
+import { TreeExpansionServiceImpl, TreeExpansionService } from '../tree-expansion';
+import { TreeNavigationService } from '../tree-navigation';
+import { TreeSearch } from '../tree-search';
+import { FuzzySearch } from '../fuzzy-search';
+import { MockLogger } from '../../../common/test/mock-logger';
+import { ILogger } from '../../../common';
+
+export function createTestTreeContainer(): Container {
+    const container = new Container({ defaultScope: 'Singleton' });
+    container.bind(TreeImpl).toSelf();
+    container.bind(Tree).toService(TreeImpl);
+    container.bind(TreeSelectionServiceImpl).toSelf();
+    container.bind(TreeSelectionService).toService(TreeSelectionServiceImpl);
+    container.bind(TreeExpansionServiceImpl).toSelf();
+    container.bind(TreeExpansionService).toService(TreeExpansionServiceImpl);
+    container.bind(TreeNavigationService).toSelf();
+    container.bind(TreeModelImpl).toSelf();
+    container.bind(TreeModel).toService(TreeModelImpl);
+    container.bind(TreeSearch).toSelf();
+    container.bind(FuzzySearch).toSelf();
+    container.bind(MockLogger).toSelf();
+    container.bind(ILogger).to(MockLogger);
+    return container;
+}

packages/core/src/browser/tree/tree-consistency.spec.ts

@@ -0,0 +1,102 @@
+/********************************************************************************
+ * Copyright (C) 2019 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import * as assert from 'assert';
+import { injectable } from 'inversify';
+import { createTestTreeContainer } from './test/tree-test-container';
+import { TreeImpl, CompositeTreeNode, TreeNode } from './tree';
+import { TreeModel } from './tree-model';
+import { ExpandableTreeNode } from './tree-expansion';
+
+@injectable()
+class ConsistencyTestTree extends TreeImpl {
+
+    public resolveCounter = 0;
+
+    protected async resolveChildren(parent: CompositeTreeNode): Promise<TreeNode[]> {
+        if (parent.id === 'expandable') {
+            const step: () => Promise<TreeNode[]> = async () => {
+                // a predicate to emulate bad timing, i.e.
+                // children of a node gets resolved when a root is changed
+                if (this.root && this.root !== parent.parent) {
+                    this.resolveCounter++;
+                    return [];
+                } else {
+                    await new Promise(resolve => setTimeout(resolve, 10));
+                    return step();
+                }
+            };
+            return step();
+        }
+        return super.resolveChildren(parent);
+    }
+
+}
+
+/**
+ * Return roots having the same id, but not object identity.
+ */
+function createConsistencyTestRoot(rootName: string): CompositeTreeNode {
+    const children: TreeNode[] = [];
+    const root: CompositeTreeNode = {
+        id: 'root',
+        name: rootName,
+        parent: undefined,
+        children
+    }
+    const parent: ExpandableTreeNode = {
+        id: 'expandable',
+        name: 'expandable',
+        parent: root,
+        expanded: true,
+        children: []
+    };
+    children.push(parent)
+    return root;
+}
+
+describe('Tree Consistency', () => {
+
+    it('setting different tree roots should finish', async () => {
+        const container = createTestTreeContainer();
+        container.bind(ConsistencyTestTree).toSelf();
+        container.rebind(TreeImpl).toService(ConsistencyTestTree);
+        const tree = container.get(ConsistencyTestTree);
+
+        const model = container.get<TreeModel>(TreeModel);
+
+        model.root = createConsistencyTestRoot('Foo');
+        await new Promise(resolve => setTimeout(resolve, 50));
+
+        model.root = createConsistencyTestRoot('Bar');
+        await new Promise(resolve => setTimeout(resolve, 50));
+
+        let resolveCounter = tree.resolveCounter;
+        assert.notStrictEqual(resolveCounter, undefined);
+        for (let i = 0; i < 10; i++) {
+            await new Promise(resolve => setTimeout(resolve, 50));
+            if (resolveCounter === tree.resolveCounter) {
+                assert.deepStrictEqual('Bar', model.root!.name);
+                return;
+            }
+            resolveCounter = tree.resolveCounter;
+        }
+        assert.ok(false, 'Resolving does not stop, attempts: ' + tree.resolveCounter);
+    });
+
+    // TODO test with recursive expansion, it should be complete
+
+});

packages/core/src/browser/tree/tree.spec.ts

@@ -15,19 +15,11 @@
  ********************************************************************************/
 
 import * as assert from 'assert';
-import { TreeNode, CompositeTreeNode, TreeImpl, Tree } from './tree';
-import { TreeModel, TreeModelImpl } from './tree-model';
+import { TreeNode, CompositeTreeNode } from './tree';
+import { TreeModel } from './tree-model';
 import { MockTreeModel } from './test/mock-tree-model';
 import { expect } from 'chai';
-import { Container } from 'inversify';
-import { TreeSelectionServiceImpl } from './tree-selection-impl';
-import { TreeSelectionService } from './tree-selection';
-import { TreeExpansionServiceImpl, TreeExpansionService } from './tree-expansion';
-import { TreeNavigationService } from './tree-navigation';
-import { TreeSearch } from './tree-search';
-import { FuzzySearch } from './fuzzy-search';
-import { MockLogger } from '../../common/test/mock-logger';
-import { ILogger } from '../../common';
+import { createTestTreeContainer } from './test/tree-test-container';
 
 // tslint:disable:no-unused-expression
 describe('Tree', () => {
@@ -239,20 +231,7 @@ describe('Tree', () => {
     }
 
     function createTreeModel(): TreeModel {
-        const container = new Container({ defaultScope: 'Singleton' });
-        container.bind(TreeImpl).toSelf();
-        container.bind(Tree).toService(TreeImpl);
-        container.bind(TreeSelectionServiceImpl).toSelf();
-        container.bind(TreeSelectionService).toService(TreeSelectionServiceImpl);
-        container.bind(TreeExpansionServiceImpl).toSelf();
-        container.bind(TreeExpansionService).toService(TreeExpansionServiceImpl);
-        container.bind(TreeNavigationService).toSelf();
-        container.bind(TreeModelImpl).toSelf();
-        container.bind(TreeModel).toService(TreeModelImpl);
-        container.bind(TreeSearch).toSelf();
-        container.bind(FuzzySearch).toSelf();
-        container.bind(MockLogger).toSelf();
-        container.bind(ILogger).to(MockLogger).inSingletonScope();
+        const container = createTestTreeContainer();
         return container.get(TreeModel);
     }
     function retrieveNode<T extends TreeNode>(id: string): Readonly<T> {