laverdet / isolated-vm

Secure & isolated JS environments for nodejs
ISC License
2.14k stars 152 forks source link

Can't build for electron on Windows #132

Closed artch closed 4 years ago

artch commented 4 years ago

We're having issues with building isolated-vm for electron on Windows. We're trying electron 4.x branch since it ships nodejs 10. This is what I get when trying to build isolated-vm 2.0.1 against electron 4.2.12 using MSBuild 2017:

C:\Users\chivc\isolated-vm>node-gyp rebuild --target=4.2.12 --arch=x64 --dist-url=https://atom.io/electron/headers
gyp info it worked if it ends with ok
gyp info using node-gyp@6.0.0
gyp info using node@10.16.3 | win32 | x64
gyp info find Python using Python version 2.7.17 found at "C:\Python27\python.exe"
gyp info find VS using VS2017 (15.9.28307.905) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Python27\python.exe
gyp info spawn args [ 'C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\isolated-vm\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\AppData\\Local\\node-gyp\\Cache\\4.2.12\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\chivc\\AppData\\Local\\node-gyp\\Cache\\4.2.12',
gyp info spawn args   '-Dnode_gyp_dir=C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\\\Users\\\\chivc\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\4.2.12\\\\<(target_arch)\\\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\Users\\chivc\\isolated-vm',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\Users\\chivc\\isolated-vm\\build',
gyp info spawn args   '-Goutput_dir=.' ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe
gyp info spawn args [ 'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64' ]
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
  external_copy_nortti.cc
  win_delay_load_hook.cc
  nortti.vcxproj -> C:\Users\chivc\isolated-vm\build\Release\\nortti.lib
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  allocator.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  class_handle.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  environment.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  holder.cc
  inspector.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  stack_trace.cc
  three_phase_task.cc
  context_handle.cc
  external_copy.cc
  external_copy_handle.cc
  isolate.cc
  isolate_handle.cc
  lib_handle.cc
  native_module_handle.cc
  reference_handle.cc
  script_handle.cc
  module_handle.cc
  session_handle.cc
  transferable.cc
  win_delay_load_hook.cc
     Creating library C:\Users\chivc\isolated-vm\build\Release\isolated_vm.lib and object C:\Users\chivc\isolated-vm\bu
  ild\Release\isolated_vm.exp
environment.obj : error LNK2001: unresolved external symbol "public: static class v8::Platform * __cdecl v8::internal::
V8::GetCurrentPlatform(void)" (?GetCurrentPlatform@V8@internal@v8@@SAPEAVPlatform@3@XZ) [C:\Users\chivc\isolated-vm\bui
ld\isolated_vm.vcxproj]
inspector.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: static class std::unique_ptr<c
lass v8_inspector::V8Inspector,struct std::default_delete<class v8_inspector::V8Inspector> > __cdecl v8_inspector::V8In
spector::create(class v8::Isolate *,class v8_inspector::V8InspectorClient *)" (__imp_?create@V8Inspector@v8_inspector@@
SA?AV?$unique_ptr@VV8Inspector@v8_inspector@@U?$default_delete@VV8Inspector@v8_inspector@@@std@@@std@@PEAVIsolate@v8@@P
EAVV8InspectorClient@2@@Z) [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
C:\Users\chivc\isolated-vm\build\Release\isolated_vm.node : fatal error LNK1120: 2 unresolved externals [C:\Users\chivc
\isolated-vm\build\isolated_vm.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\chivc\AppData\Roaming\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:248:12)
gyp ERR! System Windows_NT 10.0.18362
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild" "--target=4.2.12" "--arch=x64" "--dist-url=https://atom.io/electron/headers"
gyp ERR! cwd C:\Users\chivc\isolated-vm
gyp ERR! node -v v10.16.3
gyp ERR! node-gyp -v v6.0.0
gyp ERR! not ok

Building with local Node.js 10.16.3 works fine. Building for electron on Linux also works fine.

Note: there is a small unrelated problem with node-gyp 6.0.0 which we had to overcome using this workaround.

laverdet commented 4 years ago

Did this work before on an earlier version of Electron or is it possible this has always been broken?

Could you give this diff a shot?

diff --git a/src/isolate/platform_delegate.h b/src/isolate/platform_delegate.h
index d6c2a97..ab031dd 100644
--- a/src/isolate/platform_delegate.h
+++ b/src/isolate/platform_delegate.h
@@ -86,7 +86,7 @@ class PlatformDelegate : public v8::Platform {
                PlatformDelegate(v8::Isolate* node_isolate, v8::Platform* node_platform) : node_isolate(node_isolate), node_platform(node_platform) {}

                static PlatformDelegate& DelegateInstance() {
-                       static PlatformDelegate delegate(v8::Isolate::GetCurrent(), v8::internal::V8::GetCurrentPlatform());
+                       static PlatformDelegate delegate(v8::Isolate::GetCurrent(), node::GetMainThreadMultiIsolatePlatform());
                        return delegate;
                }
artch commented 4 years ago

With this diff now there is only one error:

inspector.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: static class std::unique_ptr<class v8_inspector::V8Inspector,struct std::default_delete<class v8_inspector::V8Inspector> > __cdecl v8_inspector::V8Inspector::create(class v8::Isolate *,class v8_inspector::V8InspectorClient
 *)" (__imp_?create@V8Inspector@v8_inspector@@SA?AV?$unique_ptr@VV8Inspector@v8_inspector@@U?$default_delete@VV8Inspector@v8_inspector@@@std@@@std@@PEAVIsolate@v8@@PEAVV8InspectorClient@2@@Z) [C:\Home\screeps\driver\node_modules\isolated-vm\build\isolated_vm.vcxproj]
C:\Home\screeps\driver\node_modules\isolated-vm\build\Release\isolated_vm.node : fatal error LNK1120: 1 unresolved externals [C:\Home\screeps\driver\node_modules\isolated-vm\build\isolated_vm.vcxproj]

The last working version was isolated-vm@1.5.2 with electron@2.0.0-beta.7, but we didn't try to build any intermediates.

laverdet commented 4 years ago

Ok I created a new electron branch for you which removes the inspector support that is bothering electron. I don't have Windows set up at the moment so it's a bit of a guessing game, but I hope that one will work.

artch commented 4 years ago

That worked, thanks for the super quick response!

artch commented 4 years ago

Well no. Build worked, but the module doesn't load. This code hangs up and doesn't output anything after loading:

console.log('loading');
try {
  require('isolated-vm');
}
catch(e) {
  console.error(e);
}
console.log('loaded');
laverdet commented 4 years ago

I took a look at this for a while today and I think it's going to be tough to get this into Electron with the changes that they've made to their build process. I need access to an internal v8 API that they've stripped from their binaries.

There's two options for now: a) Include a plain node binary with screeps that runner will run under. b) Ship a custom build of Electron with the extra feature reenabled.

I'm also going to reach out the nodejs team on #133 about the issue because I think it's a more general problem than one that just affects isolated-vm.

artch commented 4 years ago

OK, thanks, we'll have to consider it. Meanwhile, it looks like electron 2.0.18 builds and works well with isolated-vm 1.7.5, so we'll continue shipping it.