Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Awaiting promises returned by systeminformation can still block running thread #873

Open
sylt opened this issue Nov 27, 2023 · 6 comments · May be fixed by #948
Open

Awaiting promises returned by systeminformation can still block running thread #873

sylt opened this issue Nov 27, 2023 · 6 comments · May be fixed by #948

Comments

@sylt
Copy link

sylt commented Nov 27, 2023

Describe the bug
Despite that all calls to the library return promises, the promises themselves are blocking, defeating much of the purpose of having them in the first place.

The reason they are blocking is because the implementation of systeminformation uses calls like execSync(), instead of just exec() (and then awaiting the result).

To Reproduce
Use-case: Fetching WiFi info running in the electron main process.
Steps to reproduce the behavior:

  1. Issue await si.wifiNetworks(), assuming it will take long time (several seconds).
  2. While waiting for the info, the thread awaiting the network info is blocked from doing other work (even though promises are used).

Expected behavior
I expect that awaiting the result shouldn't be really blocking the thread, just making it available for other tasks to run while awaiting the result.

Environment (please complete the following information)

  • systeminformation package version: 5.21.18
  • OS: Linux

Additional information
To always reproduce the problem, simply modify an existing command to something like execSync('sleep 10;' + originalCmd).

Workaround
Put the offending systeminformation call in a worker thread, which can be properly awaited without blocking anything.

@KristjanESPERANTO
Copy link

KristjanESPERANTO commented Jan 14, 2024

@sylt You probably mean si.wifiNetworks() instead of si.getWifiNetworks(), right?

I can confirm this issue with si.wifiNetworks(), si.system() and si.osInfo(), but only with an electron app on arm64.

A simple node script on arm64 works.
An electron app on x64 works too.

@sylt
Copy link
Author

sylt commented Jan 14, 2024

@KristjanESPERANTO: Yes, of course, thank you :) I have corrected the description!

And just to show-case the difference between using execSync and exec, here's an example program when starting a 100 ms timer, but using different strategies depending on if useSync is true or false:

const child_process = require("child_process")
const process = require("process")

const now = new Date();
setTimeout(() => {console.log(new Date() - now + " ms elapsed")}, 100)

const useSync = true;

process.nextTick(async () => {
    if (useSync) {
        child_process.execSync("sleep 2");
    }
    else {
        await child_process.exec("sleep 2");
    }    
})

As can be seen, the timer is prevented from running if we're issuing execSync(), but is allowed to run when calling await exec().

@linuswillner
Copy link

I'm hitting this same behaviour with Electron v33.0.2, on macOS ARM64. It would be nice to have this problem fixed, because this library has a lot of obscure edge cases already handled, and I'd rather not go through all the work of rewriting the functions from this library just to get rid of this one problem...

@sylt you mentioned that a workaround is to put the offending systeminformation call inside a worker thread to cut the problem down to size. You don't happen to have any code still laying around for it?

@sylt
Copy link
Author

sylt commented Nov 14, 2024

@linuswillner Sure! Here's the essence of it, using threads.js (see docs at https://threads.js.org/ for more examples).

Below is the "main.ts", running in the main thread (doing the await):

import { spawn, Worker } from 'threads';

async function someFunctionRunningInTheMainThread()
{
  const getNetworkInfo = await spawn(new Worker('./networkinfo'));
  try {
    const [wifiConnections, networkInterfaces] = await getNetworkInfo();

    // do stuff with network information requested above
    ...
  } catch (...) {
    // ...
  }
}

Below is file "networkinfo.ts", which gets run in a separate worker thread, exposing a minimal API for getting the requested information:

import { expose } from 'threads/worker';
import * as si from 'systeminformation';

expose(async function getNetworkInfo() {
  return Promise.all([si.wifiConnections(), si.networkInterfaces()]);
});

In our specific case, it's not more complicated than this. I hope it's possible for you to tailor it to your needs!

@linuswillner
Copy link

Fantastic, thank you! 🙏

@dcodeIO
Copy link

dcodeIO commented Nov 22, 2024

Addressed by

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants