Symbol(LanguageServerContribution) not always unique, breaking bindings

[westbury]

There appears to be a problem with the symbol used for binding LanguageServerContribution when built using Theia modules from npm. The problem can be seen in the theia-ide/theia-xtext project. The DSL backend never has the 'start' function called.

If one puts a break point in, say, LanguageBackendContribution.configure, and expand this.contributors until one sees the map entries, one will see that there are two entries with a key of Symbol(LanguageServerContribution). One has an array with just the DSL language extension, and the other has the other five language extensions.

If one builds the Theia modules alongside the DSL LSP module then the problem does not occur, there is a single entry in Inversify's map with all six extensions and all is fine. This problem occurs on Windows, I have not been able to test this on Linux. (Note that to build on Windows you will have to remove the bash commands in the yarn scripts, or use the fork at westbury/theia-xtext).

[svenefftinge]

This happens if you have pulled multiple different versions of the same package in. Please check whether you have any nested @theia extensions in node modules.

[westbury]

I don't believe that pulling multiple versions is the cause here. I can reproduce this with a clean clone of the repository with no changes (thank you for merging the changes to enable it to build on Windows, by the way). I build the jar, yarn install, and debug using the given launch configuration, and I see the problem.

The directory listings are:

` Directory of C:\Users\nigwes01\git\theia-xtext\app\node_modules

11/21/2018 11:31 PM

. 11/21/2018 11:31 PM .. 11/21/2018 11:31 PM .bin 0 File(s) 0 bytes 3 Dir(s) 69,435,994,112 bytes free

C:\Users\nigwes01\git\theia-xtext>dir app\node_modules.bin Volume in drive C is OSDisk Volume Serial Number is A0A6-2BF0

Directory of C:\Users\nigwes01\git\theia-xtext\app\node_modules.bin

11/21/2018 11:31 PM

. 11/21/2018 11:31 PM .. 11/21/2018 11:31 PM 349 theia 11/21/2018 11:31 PM 226 theia.cmd 2 File(s) 575 bytes 2 Dir(s) 69,435,994,112 bytes free

C:\Users\nigwes01\git\theia-xtext>dir xtext-dsl-extension\node_modules Volume in drive C is OSDisk Volume Serial Number is A0A6-2BF0

Directory of C:\Users\nigwes01\git\theia-xtext\xtext-dsl-extension\node_modules

11/21/2018 11:31 PM

. 11/21/2018 11:31 PM .. 11/21/2018 11:31 PM .bin 0 File(s) 0 bytes 3 Dir(s) 69,435,990,016 bytes free

C:\Users\nigwes01\git\theia-xtext>dir xtext-dsl-extension\node_modules.bin Volume in drive C is OSDisk Volume Serial Number is A0A6-2BF0

Directory of C:\Users\nigwes01\git\theia-xtext\xtext-dsl-extension\node_modules.bin

11/21/2018 11:31 PM

. 11/21/2018 11:31 PM .. 11/21/2018 11:31 PM 347 copyfiles 11/21/2018 11:31 PM 224 copyfiles.cmd 11/21/2018 11:31 PM 347 copyup 11/21/2018 11:31 PM 224 copyup.cmd 11/21/2018 11:31 PM 345 tsc 11/21/2018 11:31 PM 222 tsc.cmd 11/21/2018 11:31 PM 355 tsserver 11/21/2018 11:31 PM 232 tsserver.cmd 8 File(s) 2,296 bytes 2 Dir(s) 69,435,990,016 bytes free `

[westbury]

I suspect this issue would be fixed by using Symbol.for. See for example https://github.com/inversify/InversifyJS/issues/691 . I will test this out when I get time.

[akosyakov]

@westbury you load the same symbol twice at runtime from different js modules, it can happen only if you have the same extension loaded several times. Symbol.for is not a solution, because it will bogusly use an implementation from one package for clients written against another.

You should find a root cause of why a symbol is loaded twice. Please start by running yarn why @theia/languages to check whether you indeed have multiple extensions.

[akosyakov]

If one puts a break point in, say, LanguageBackendContribution.configure, and expand this.contributors until one sees the map entries, one will see that there are two entries with a key of Symbol(LanguageServerContribution). One has an array with just the DSL language extension, and the other has the other five language extensions.

i could not understand which map entries do you mean, sorry. What I see when i debug LanguageBackendContribution:

I was able to debug your fork as well. Don't see any duplicate symbols and start is properly called when a dsl file is opened:

[akosyakov]

Inspecting all bindings also returns only one match for Symbol(LanguageServerContribution):

expression: [..._this.contributors.container._bindingDictionary._map.entries()].filter(([key]) => key === "Symbol(LanguageServerContribution)")

[westbury]

Note that this issue occurs on Windows only. On Linux one sees Symbol(LanguageServerContribution) once in the map, on Windows one sees it twice in the map (both using clean clones of the theia-xtext project).

[westbury]

This issue shows whenever debugging a downstream project on Windows. For another example see this discussion: https://spectrum.chat/theia/general/unable-to-debug-backend-in-a-theia-extension~b91641ab-5c1a-41a7-a163-ee0b7dbaccbe

Debugging this example on Windows, exactly as cloned from https://github.com/cherxp/frontend-backend-theia-extn, and looking at the contribution map we see Symbol(ConnectionHandler) at position 9 with 10 contributions:

And scrolling further down the map we see another Symbol(ConnectionHandler), this one with just the TestServerImpl from the downstream project:

The inversify code in lookup.js is:

var entry = this._map.get(serviceIdentifier);
        if (entry !== undefined) {
            entry.push(value);
            this._map.set(serviceIdentifier, entry);
        }
        else {
            this._map.set(serviceIdentifier, [value]);
        }

so there's nothing more to debug. The symbol passed from the downstream project is considered by Javascript to be different from the symbol used in the Theia packages. The next step is to isolate a test case for Nodejs.

[westbury]

And doing the exact same steps on Ubuntu I see just the one entry for ConnectionHandler with all 11 contributions:

[akosyakov]

@westbury could you please run yarn why @theia/core on windows for this project and paste output here

[westbury]

The output on Windows is:

c:\Users\nigwes01\git\frontend-backend-theia-extn>yarn why @theia/core
yarn why v1.13.0
[1/4] Why do we have the module "@theia/core"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@theia/core@0.9.0"
info Reasons this module exists
   - "_project_#serverevent" depends on it
   - Hoisted from "_project_#serverevent#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#editor#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#languages#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#preferences#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#process#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#terminal#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#markers#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#messages#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#typescript#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#workspace#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#monaco#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#navigator#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#filesystem#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#typescript#@theia#callhierarchy#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#monaco#@theia#outline-view#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#languages#@theia#output#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#preferences#@theia#json#@theia#core"
   - Hoisted from "_project_#electron-app#@theia#preferences#@theia#userstorage#@theia#core"
   - Hoisted from "_project_#browser-app#@theia#workspace#@theia#variable-resolver#@theia#core"
Done in 3.54s.

The same as on Linux.

[akosyakov]

@westbury thanks, it looks good. It would be interesting to know how it happens that 2 different symbols are created for the symbol contributed from the same file. Would it be possible to put a breakpoint here and see why this file is loaded twice and not reused second time from the cache? According to https://nodejs.org/api/modules.html#modules_caching it should not happen. Could it be that symlinking involved and the same file over symlinks looks different for Node.js?

[westbury]

The breakpoint is hit just once:

[westbury]

I was wondering if anyone has a Mac they can try this on. The reason I ask is that articles have mentioned the problems caused by a case-insensitive file system on the symbol cache, and the nodejs.org link above also mentioned this. I don't know how the case insensitivity could explain what we are seeing but knowing if the problem occurs on the Mac might help hone in on the cause.

[westbury]

Could it be that symlinking involved and the same file over symlinks looks different for Node.js?

@akosyakov It appears you are absolutely correct. I replaced the symbolic links (browser-app, electron-app, and serverevent in nodemodules) with copies of the directories and it worked correctly, one symbol with all 11 contributions.

[westbury]

Setting the --preserve-symlinks arg to node fixes the problem. In the launch configuration the env setting is NODE_PRESERVE_SYMLINKS, so change 'env' in the launch configuration in the frontend-backend-theia-extn example with:

            "env": {
                "NODE_ENV": "development",
                "NODE_PRESERVE_SYMLINKS": "1"
            },

[akosyakov]

@westbury good that we have a workaround, but so strange that it is required

From docs:

By default, when Node.js loads a module from a path that is symbolically linked to a different on-disk location, Node.js will dereference the link and use the actual on-disk "real path" of the module as both an identifier and as a root path to locate other dependency modules.

It sounds to me that all symlinks should be resolved the real path and load the same file. It seems to work on all OS, except windows? Maybe we should open an issued for nodejs to clarify.

[westbury]

Yes, it certainly seemed backwards to me. I would expect the default to work and setting the 'preserve-symlinks' to make it fail. I certainly think you are right to call this a workaround and not a fix.

[kittaakos]

Closing. @theia/languages has been removed.