Implement process.kill() function

[stefansundin]

Proposed changes

This PR implements the process.kill() function, using the same API as Node.js. https://nodejs.org/api/process.html#processkillpid-signal

My use case for this function is not to kill processes, but to send other signals, like SIGHUP. But the default is to send SIGTERM, just like in Node.js.

I am not 100% sure that I am using the correct string functions to compare the input with the signal names. It is working but I feel like it is not the proper way to do it. Please review the patch and let me know what the proper way of doing things are.

I am using #ifdef for almost all signal names, except for SIGTERM. Not sure if it is strictly necessary but I don't think all signals are defined for all platforms?

I tested this on Debian 12.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

• I have read the CONTRIBUTING document
• If applicable, I have added tests that prove my fix is effective or that my feature works
• If applicable, I have checked that any relevant tests pass after adding my changes

[xeioex]

Hi @stefansundin,

Thank you for the patch. We need the QJS implementation as well for shell tests to pass. I will do that myself, after polishing you patch.

For signals, I propose to use only standard signals from man 7 signal up to P1990. With that we can get rid of ifdefs.
For else if blocks, may be the better way is to use a table. Something like this.

[stefansundin]

Thank you @xeioex. I really appreciate the assistance.

I have pushed a new commit that I think incorporates your suggestions. Please let me know if there's anything you want me to change.

I can squash the commits if you prefer. Just let me know.

[xeioex]

@stefansundin

Thank you for the fast response.

1. Looking at we we have now, I think we can remove "SIG" part from the table, so we do not check it again and again. Instead we can check for "SIG" just once, before the table.

2. Please, change // P1990 signals from man 7 signal are supported
   to /* ... */ as we do not use //-style comments.

3. Also, feel free to squash all the changes into a single patch.

[stefansundin]

I think I managed to do it correctly. It got a little bit tricky with the pointers since I don't think there is a helper function to do these comparisons. But it is working and there are length checks so I think it is secure.

[xeioex]

Hi @stefansundin,

I added QuickJS part. I also added minor improvements and Shell: added process.kill() test patches.
Please test them, and if you like them squash to your original commit.

[stefansundin]

I reviewed it and it looks great. 👍

I rebuilt my project and tested with the new changes and it worked.

I tried to build with QuickJS to test that but I encountered a compilation issue (building in docker):

37.66 /usr/bin/ld: /root/quickjs/libquickjs.a(quickjs.o): relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `js_realloc_rt' which may bind externally can not be used when making a shared object; recompile with -fPIC
37.66 /root/quickjs/quickjs.c:1458:(.text+0x3fc90): dangerous relocation: unsupported relocation

In any case, I squashed the commits since I think you probably tested QuickJS. :)

[xeioex]

@stefansundin

I tried to build with QuickJS to test that but I encountered a compilation issue (building in docker):

Something like CFLAGS='-fPIC' make libquickjs.a should fix the problem.

In any case, I squashed the commits since I think you probably tested QuickJS. :)

Thank you for the contribution. Next week @VadimZhestikov will do the review of QuickJS patches, and will merge the PR. I will also unsquash my QuickJS commits.

[stefansundin]

Thanks for the -fPIC tip, I was able to built QuickJS with it. 👍

However, I think think there may be an issue in the QuickJS implementation, I got this error when attempting to call process.exit():

[error] 239#239: *1 js call exception: ReferenceError: 'process' is not defined

[VadimZhestikov]

Looks good

[xeioex]

@stefansundin Thank you, committed

[stefansundin]

Nice!! I just retested QuickJS and I can confirm that the error I posted above no longer happens and the signal is sent correctly. 👍