Open mikedotexe opened 2 years ago
It's possible that Vadim's setup was related to VSCode, as we discussed in our meeting, but still unsure.
I want to add a little on that - this is not related to VSCode, but rather that's sandbox nodes spawning and spinning even after the test is finished. My teammate has a macbook with 16 gigs of RAM and after running a bunch of tests all the memory is full and you need either to restart your machine or kill those node processes manually.
I also suggest changing the name of the issue to something with "memory usage" in it.
@adsick what bout the newest version? Do you still experience high memory usage?
@volovyk-s we've switched to workspaces-rs, so I can't tell if the problem remains.
I want to add a little on that - this is not related to VSCode, but rather that's sandbox nodes spawning and spinning even after the test is finished. My teammate has a macbook with 16 gigs of RAM and after running a bunch of tests all the memory is full and you need either to restart your machine or kill those node processes manually.
@ailisp I'm permanently facing this problem. Background processes are draining batteries and using computer resources. Right now I literally need to close them in Activity Monitor or reboot.
@ChaoticTempest are you experiencing something like this in Rust version? How are you making sure that the process is killed if the test fails?
I recalled using python subprocess, there's a libc function prctl, and set PR_SET_PDEATHSIG flag at child process creation can make sure SIGKILL signal is sent to child process when parent process (python) exit. Threoratically we can apply same way in starting sandbox with node ffi, but I didn't see any example.
Or we can provide a function to detect and shutdown sandbox processes, and try to call them in ava after.always
@ailisp afterEach.always is a good solution for it: https://github.com/near/workspaces-js/pull/194
@volovyks if the test fails, the process gets cleaned up since the memory gets dropped. Since it's in a compiled language, it figures out where to add drops if panics
happen, so we're good in cases where the program crashes
Howdy folks -- just noticed this issue and it tickled a bit - looking for some clarification.
Do I understand correctly that the child processes that we spawn from our workspaces-js
code aren't being closed when the node
process shuts down after a test run, unless we have called helper methods from the test framework to make that happen?
If so, then IMO the .always
usage is actually a workaround rather than a bugfix; we shouldn't require our consumers to handle this case. It's possible that not all test frameworks people will use will support something like the .always
functionality that ava
provides, and they will have different APIs even if they do, which will make our documentation to avoid this behaviour framework-specific, which will become a maintenance and discoverability nightmare.
It's actually pretty straightforward to implement, but rather than write our own code to gracefully handle this case, IMO we should delegate process creation to the execa
package, which has this functionality built-in -- along with a lot of other nice benefits like cross platform support for Windows :)
https://github.com/sindresorhus/execa https://www.npmjs.com/package/execa (49 million weekly downloads :))
If you're curious about the how it works internally: https://github.com/sindresorhus/execa/blob/770788027308cb8a5d2002445c65e25024e1a4c6/lib/kill.js#L90
@MaximusHaximus execa
is similar to what we have in Workspaces RS. Yes, it can make our logic bulletproof for memory leaks (for this issue), but it can take away some flexibility. Sometimes I comment.after()
to continue work with sandox from the CLI even after the test process is dead. Not sure if this is a common scenario.
I received this feedback from devs, and while I cannot provide more information than this, think it's useful to store in the GitHub issues.