Use delayed loading for windows

[jrd-rocks]

Electron on windows needs to use delayed loading. For node.js delayed loading works as well (it's the default when building with node.gyp)

For full details see https://github.com/jrd-rocks/neon/blob/electron_delay_hook/README%20ELECTRON%204.md #389

The neon command that creates a default project still needs fixing to add a .cargo/config file with the linker command. Never mind that, i updated the cli script as well (a breeze after figuring out the linker delay load black magic)

[amilajack]

I think it might be better to add README ELECTRON 4.md to a wiki

[jrd-rocks]

I think it might be better to add README ELECTRON 4.md to a wiki

I think it should be part of the documentation on the website, but couldn't find where to change that.

[amilajack]

Oh yea that's even better. I'm the maintainer of the docs so I'll PR there after this is merged.

[bnoordhuis]

There should probably be an opt-out. See bnoordhuis/node-heapdump@78056ec1 for an example where the delayed loading hook had to be disabled.

[bnoordhuis]

You don't need to export the symbol, only __pfnDliNotifyHook2 needs to be exported. Just this should suffice:

static FARPROC WINAPI __load_exe_hook(unsigned int event, DelayLoadInfo* info) {
  // ...
}

decltype(__pfnDliNotifyHook2) __pfnDliNotifyHook2 = __load_exe_hook;

By the way, I think you could turn this into a .rs file. You'd probably have to provide definitions of dliNotePreLoadLibrary and DelayLoadInfo though, so it might be not be shorter.

[jrd-rocks]

Much better, thanks!

I have no idea how to write those definitions they're pretty complicated to me. And as neon-runtime already includes c code anyways, i figured a little more shouldn't be a problem. Though of course pure rust is nicer.

Optionally removing it is as simple as removing .cargo/config (though the hook then still gets needlessly compiled in neon-runtime) as long as it's documented it's there i think it shouldn't be an issue.

[itserg2008]

Hello @dherman , do you have any plans to review and merge it to the master branch in the nearest time?

[kjvalencik]

@jrd-rocks Thanks for this contribution! Are you still interested in working on this? If so, I have some notes on simplifications. Thanks, again!

• Omit building win_delay_load_hook.obj, it's already built by node-gyp in neon-runtime
• Add a method to neon-runtime/build.rs to copy win_delay_load_hook.obj to OUT_DIR
• Detect compiling for windows in the neon-cli by checking npm_config_runtime=electron in target.ts
• If compiling for electron and on windows, use the RUSTFLAGS environment variable to link in the delay

env: {
  ...process.env,
  RUSTFLAGS: '-C link-arg=win_delay_load_hook.obj -C link-arg=delayimp.lib -C link-arg=/DELAYLOAD:node.exe'
}

I've got a hacked together branch and validates this approach works, but it needs to be done properly.

https://github.com/kjvalencik/neon/tree/windows-electron

[joshuef]

I was trying to get this going, just to be clear: I'll need to use the windows-electron fork as a dep in our neon app, but also build the app on windows using the forked CLI too, right? Should anything else be needed to test this?

I'll be happy to try and dive into neon to try and get the needed changes in assuming i can get them going (so far still seeing module did not self load :| )

[kjvalencik]

@joshuef Yes, you would also need to use the electron-build-env wrapper. I would not expect this on the fork since it is working locally and in CI. (The fork should still be working, but I just broke the upstream branch testing something).

[kjvalencik]

@joshuef Are you on the neon Slack? I'm not a windows developer and it would be helpful if you are able to test my work. Thanks!