Support Multiple Apps Running with IOHook #294

chevonc · 2021-02-08T19:51:15Z

Description

Port LibUIOHook changes to use non-shareable synchronization primitive kwhat/libuiohook#72

Motivation and Context

Allows multiples app running with IOHook to use the library without lagging windows OS due to mutex contention.
Fixes #93

How Has This Been Tested?

Yes, compiled locally and tested with on Windows 10 by spawning two processes importing IOHook. System no longer freezes or lags.

Screenshots (if appropriate):

IOHook.Input.Lag.Fix.mov

Types of changes

[] Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

special thanks to @Wizek for iohook-test harness

WilixLead · 2021-02-09T13:46:41Z

Hi @chevonc! Your video looks very cool. So we need to review PR and fix build steps in Appveyor (looks like problems with Nan or Node version)
I do fast review and looks like you change only mutex parts by changing it to critical sections and this solution affect only windows systems?

chevonc · 2021-02-09T16:50:37Z

@WilixLead yes, it should only affect windows systems.

Looking at the build logs, it seems Appveyor's only errors are with node 8.9.3 & 9.2.0. I think this is because the node-gyp build headers + windows sdk libs used when targeting node 8.9.3, abi 57 or node 9.2.0, abi 59 don't support the synchapi.h imports used in this change :/.

I can attempt to see if there's a way around this, but my hunch is removing the target these nodes versions from the package.json is the best fix here. Official LTS (long term support) for node 8 ended last yr (and 9 never had LTS). I think this is critical fix being held back by an old node versions. I'll go ahead and remove the targets, lmk what you think.

marcelblum · 2021-02-12T22:18:13Z

Tests working well in Windows 10/Electron 11. Also no conflict if another instance of previous version iohook is running. Nice!

chevonc · 2021-02-18T17:41:59Z

@WilixLead @Djiit any chance to get a review/feedback here? Hoping this could be merged soonish, so I can pull the updated package

Djiit · 2021-03-15T10:19:30Z

Hey, just passing by, LGTM, what do you think @WilixLead ?

Djiit · 2021-03-15T10:20:05Z

Remember to make it a BREAKING change since we remove support for old version (8/9)

WilixLead · 2021-03-15T11:49:18Z

Node 8/9 still very used and this PR not tested with Windows less then 10 (7/8), but I think older Windows versions should work well.
Also this is breaking change, we should bump major version. As of I skipped part of project life, @Djiit please say what you think? Bump version and accept PR ?

I review code, looks ok (I don't have Windows machine at this moment, but I can try later)

chevonc · 2021-03-18T17:00:28Z

I bumped the version major version to signify this is breaking change (ie - node 8/9 no longer supported)

chevonc · 2021-03-25T21:24:55Z

@Djiit @WilixLead any chance we can get some movement on this? I bumped the major version, but let me know if there's something else blocking it

Djiit · 2021-03-25T21:32:39Z

Sorry for the late reply. I would advise to bump only a minor, as we are pre 1.0, following semver. @WilixLead would you please merge this, set the correct version on master and release a new version on npm?

chevonc · 2021-03-31T01:54:42Z

@WilixLead any chance we can get this merged/released this wk, please? Let me know if I can assist in any way

Wizek · 2021-04-02T18:35:35Z

@chevonc in case they are reluctant/indifferent to merge, maybe we can publish it on npm as iohook-fixed/nonfreezing or something. Let me know if you want my help in that.

chevonc · 2021-04-05T02:54:16Z

@Wizek seems like that may be the fastest route here. Happy to take your assistance here to compile/publish these changes to npm if you wanna go ahead and take a stab.

Wizek · 2021-04-05T12:28:09Z

@chevonc So, AFAIR, publishing a js-only package on npm is quite easiy, e.g. like this: https://zellwk.com/blog/publish-to-npm/

The only difficulty might be the C binary. The way the willix package does it is that they pre-build and upload it to a GitHub release like this: https://github.com/wilix-team/iohook/releases/tag/undefined0.7.2 and during npm install, they try to download from here the appropriate version.

If that would fail, there perhaps should be a fallback to build on the local system of the installing user. However, for some reason I ran into some issues when I got to this point.

C:\Users\Wizek\sandbox\iohook-test> npm i github:Wizek/iohook#multiappsPatch

C:\Users\Wizek\sandbox\iohook-test> npm i github:Wizek/iohook#multiappsPatch
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated [email protected]: this library is no longer supported

> [email protected] install C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook
> node install.js

Downloading prebuild for platform: iohook-v1.0.0-node-v83-win32-x64
Downloading prebuild.tar.gz
Error: GET https://github.com/wilix-team/iohook/releases/download/v1.0.0/iohook-v1.0.0-node-v83-win32-x64.tar.gz returned 404
Prebuild for current platform (iohook-v1.0.0-node-v83-win32-x64) not found!
Try to compile for your platform:
# cd node_modules/iohook;
# npm run build

C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\install.js:16
  throw err;
  ^

[Error: ENOENT: no such file or directory, open 'C:\Users\Wizek\AppData\Local\Temp\prebuild.tar.gz'] {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'open',
  path: 'C:\\Users\\Wizek\\AppData\\Local\\Temp\\prebuild.tar.gz'
}
npm WARN iohook-test No description
npm WARN iohook-test No repository field.
npm WARN iohook-test No license field.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install: `node install.js`
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Wizek\AppData\Roaming\npm-cache\_logs\2021-03-31T09_11_51_955Z-debug.log

Then:

C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook> npm run build

C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook> npm run build

> [email protected] build C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook
> node build.js --upload=false

Compiling iohook for node v14.12.0>>>>
Warning: Missing input files:
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\system_properties.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\logger.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\include\uiohook.h
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\logger.h
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_hook.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\post_event.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_helper.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_helper.h
Warning: Missing input files:
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\logger.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_helper.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_hook.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\include\uiohook.h
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\logger.h
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\post_event.c
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\input_helper.h
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\..\libuiohook\src\windows\system_properties.c
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
  logger.c
c1 : fatal error C1083: Cannot open source file: '..\libuiohook\src\logger.c': No such file or directory [C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build\uiohook.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\MSBuild\15.0\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:314:20)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12)
gyp ERR! System Windows_NT 10.0.19041
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\Wizek\\sandbox\\iohook-test\\node_modules\\iohook\\node_modules\\node-gyp\\bin\\node-gyp.js" "configure" "rebuild" "--target=14.12.0" "--arch=x64" "--v8_enable_pointer_compression=1" "--msvs_version=2017"
gyp ERR! cwd C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook
gyp ERR! node -v v14.12.0
gyp ERR! node-gyp -v v7.1.2
gyp ERR! not ok
Error: Failed to build...
    at ChildProcess.<anonymous> (C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook\build.js:172:23)
    at ChildProcess.emit (events.js:314:20)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] build: `node build.js --upload=false`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Wizek\AppData\Roaming\npm-cache\_logs\2021-04-05T12_20_49_647Z-debug.log
C:\Users\Wizek\sandbox\iohook-test\node_modules\iohook>

I think there are two ways forward from this:

Perhaps you can try to help me figure out why this doesn't work.
Or perhaps much more simply: you upload one or few prebuilt binaries to https://github.com/chevonc/iohook/tree/multiappsPatch as a GitHub release and we change the install.js ever so slightly to point there instead of https://github.com/wilix-team/iohook/releases/download/v1.0.0/iohook-v1.0.0-node-v83-win32-x64.tar.gz

If either of these works I think we can publish.

Wizek · 2021-04-05T12:36:29Z

About the build fallback failure, this seems to be the culprit: https://github.com/chevonc/iohook/blob/multiappsPatch/.npmignore

I'm continuing to look into this.

bikutaa-dev · 2021-04-05T18:28:45Z

Please let me know if you manage to publish it, really want to use this library for an electron project I'm working on!

WilixLead · 2021-04-05T18:47:18Z

@bikutaa-dev Ok. Just change version to 0.9.0 and I can merge it

Wizek · 2021-04-05T21:57:58Z

@WilixLead Since I can not change code in either this repo or chevonc's, I've made the version change over here: #311

So was this the only blocking issue and that can be merged now?

chevonc · 2021-04-06T20:54:51Z

thank you for merging @WilixLead. Any chance we can get a new npm release w. this version as well? So far, I only see it on github with 'undefined' in the tag - https://github.com/wilix-team/iohook/releases/tag/undefined0.9.0

WilixLead · 2021-04-07T07:24:15Z

Looks like some bugs in string concatination 😄 So. At this moment most important for update in npm - it is remember how we doing it 😂
@Djiit Can you please remind me, how we deploy to NPM ? (I guess it should be in CI step)

Djiit · 2021-04-07T07:41:34Z

Always done this manually from my machine with a simple npm publish

WilixLead · 2021-04-07T09:36:16Z

Ok. I guess now all ok in NPM, please @Wizek @chevonc check your changes in new version

chevonc · 2021-05-12T14:23:26Z

Changes look great so far

fix multiple apps

753751b

WilixLead self-requested a review February 9, 2021 13:47

WilixLead mentioned this pull request Feb 9, 2021

Fix app freezing (fixes: #93) #145

Closed

8 tasks

drop old node version

d7e6dcc

Remove node 9 as well

16c19f7

chevonc mentioned this pull request Feb 10, 2021

Freezes when more than one app use iohook #93

Closed

bump major version

c288f9f

Wizek mentioned this pull request Apr 5, 2021

(Fixed version to 0.9.0) Support Multiple Apps Running with IOHook #311

Merged

WilixLead merged commit c288f9f into wilix-team:master Apr 6, 2021

WilixLead mentioned this pull request Apr 8, 2021

Can't use iohook on a simple node app #275

Open

ash0x0 mentioned this pull request Jun 11, 2021

iohook makes debugging in VSCode unusable under Windows #176

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Multiple Apps Running with IOHook #294

Support Multiple Apps Running with IOHook #294

chevonc commented Feb 8, 2021 •

edited

Loading

WilixLead commented Feb 9, 2021

chevonc commented Feb 9, 2021 •

edited

Loading

marcelblum commented Feb 12, 2021

chevonc commented Feb 18, 2021

Djiit commented Mar 15, 2021

Djiit commented Mar 15, 2021

WilixLead commented Mar 15, 2021

chevonc commented Mar 18, 2021

chevonc commented Mar 25, 2021

Djiit commented Mar 25, 2021

chevonc commented Mar 31, 2021

Wizek commented Apr 2, 2021 •

edited

Loading

chevonc commented Apr 5, 2021

Wizek commented Apr 5, 2021

Wizek commented Apr 5, 2021

bikutaa-dev commented Apr 5, 2021

WilixLead commented Apr 5, 2021

Wizek commented Apr 5, 2021 •

edited

Loading

chevonc commented Apr 6, 2021

WilixLead commented Apr 7, 2021

Djiit commented Apr 7, 2021

WilixLead commented Apr 7, 2021

chevonc commented May 12, 2021

Support Multiple Apps Running with IOHook #294

Support Multiple Apps Running with IOHook #294

Conversation

chevonc commented Feb 8, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

WilixLead commented Feb 9, 2021

chevonc commented Feb 9, 2021 • edited Loading

marcelblum commented Feb 12, 2021

chevonc commented Feb 18, 2021

Djiit commented Mar 15, 2021

Djiit commented Mar 15, 2021

WilixLead commented Mar 15, 2021

chevonc commented Mar 18, 2021

chevonc commented Mar 25, 2021

Djiit commented Mar 25, 2021

chevonc commented Mar 31, 2021

Wizek commented Apr 2, 2021 • edited Loading

chevonc commented Apr 5, 2021

Wizek commented Apr 5, 2021

Wizek commented Apr 5, 2021

bikutaa-dev commented Apr 5, 2021

WilixLead commented Apr 5, 2021

Wizek commented Apr 5, 2021 • edited Loading

chevonc commented Apr 6, 2021

WilixLead commented Apr 7, 2021

Djiit commented Apr 7, 2021

WilixLead commented Apr 7, 2021

chevonc commented May 12, 2021

chevonc commented Feb 8, 2021 •

edited

Loading

chevonc commented Feb 9, 2021 •

edited

Loading

Wizek commented Apr 2, 2021 •

edited

Loading

Wizek commented Apr 5, 2021 •

edited

Loading