Add Node.js executor for DMOJ #1143

pxpeterxu · 2023-12-21T17:28:35Z

Thanks again for maintaining DMOJ! It's a great project with a really solid security foundation.

Motivation

We wanted to use DMOJ for internal company events, and since our company's primary language is Javascript (using Node.js), we wanted to implement a version that used Node directly rather than using https://github.com/DMOJ/v8dmoj

This resolves issue #1136

Changes

Add a judge executor that will execute code written in Node.js
- This was based on the Coffeescript executor, except with some newer syscalls added (e.g., io_uring)

Related PRs on other repositories

Update Dockerfile to install Node.js to tier3 runtime image runtimes-docker#71: Update Dockerfile to install Node.js to tier3 runtime image

Testing

Deployed a judge server with this with the following Dockerfile:

FROM dmoj/runtimes-tier3

# Install Node from nodesource based on https://github.com/nodesource/distributions/tree/69a4558#installation-instructions
RUN apt-get install -y ca-certificates curl gnupg && mkdir -p /etc/apt/keyrings && { curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg; } && { echo 'deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_20.x nodistro main' > /etc/apt/sources.list.d/nodesource.list; } && apt-get update && apt-get install -y nodejs

# The remainder of this file is copied from
# https://github.com/DMOJ/judge-server/blob/master/.docker/tier3/Dockerfile
RUN mkdir /judge /problems && cd /judge && \
	curl -L https://github.com/wanderlog/judge-server/archive/master.tar.gz | tar -xz --strip-components=1 && \
    python3 -m venv --prompt=DMOJ /env && \
	/env/bin/pip3 install cython && \
	/env/bin/pip3 install -e . && \
	/env/bin/python3 setup.py develop && \
	HOME=~judge . ~judge/.profile && \
	runuser -u judge -w PATH -- /env/bin/dmoj-autoconf -V > /judge-runtime-paths.yml && \
	echo '  crt_x86_in_lib32: true' >> /judge-runtime-paths.yml

Verified that test runs using this Dockerfile worked properly:

dmoj-build · 2023-12-21T17:28:38Z

Can one of the admins verify this patch?

quantum5 · 2023-12-21T22:49:30Z

Your code looks fairly good, but I have to ask: what's the point of this? If you look at your submission for the A Plus B problem, you'll note that you are doing a lot of I/O boilerplate, mostly because node.js is designed for stream-based I/O and not so much for line-based I/O. You also won't be able to use npm modules on the judge.

The V8JS runtime is the exact same JS engine except it comes with a bunch of line-based I/O functions that requires a lot less boilerplate to use.

pxpeterxu · 2023-12-21T22:56:21Z

Your code looks fairly good, but I have to ask: what's the point of this? If you look at your submission for the A Plus B problem, you'll note that you are doing a lot of I/O boilerplate, mostly because node.js is designed for stream-based I/O and not so much for line-based I/O. You also won't be able to use npm modules on the judge.

Good point -- I think the main goal of this was for our engineers to run the code they submit locally on their machines without having to install the https://github.com/DMOJ/v8dmoj client locally, since Node.js is already preinstalled on their machines and is pretty widely used! It does require somewhat more boilerplate in code, though we often provide this with the problem.

I'd understand if this is too different from the goals of the main DMOJ site though; let me know what might be helpful.

quantum5

After some discussion, we've decided that Node.js is an okay addition after all.

dmoj/executors/NODEJS.py

quantum5 · 2023-12-22T22:39:48Z

dmoj/executors/NODEJS.py

+        # Start syscalls copied from COFFEE.py
+        'capget',
+        'eventfd2',
+        'newselect',
+        'sched_yield',
+        'select',
+        'setrlimit',
+        'statx',
+        # End syscalls copied from COFFEE.py


Can we try trimming down this list? Also the comment is not necessary.

Thanks for the suggestion! I ran /judge/.docker/entry test | grep NODEJS multiple times while commenting out these, and it seems like we only need capget and eventfd2. I left these two (and shutdown) in.

dmoj/executors/NODEJS.py

pxpeterxu · 2024-01-06T22:13:29Z

@quantum5 Thanks again for the review and happy new year! I think I've addressed all comments; can you please take a look?

Also, if you can check the associated PRs, that'd be great too:

README.md

Xyene · 2024-01-07T17:45:22Z

It looks like lint failed, but otherwise this looks good to me.

Once you've resolved that, could you please rebase and squash your commits? After that we should be good to merge.

murtadha · 2024-01-13T21:27:10Z

I'm eagerly waiting for this PR to be merged as our community has also been asking for the ability to run JS code via Node.js for exactly the same reason @pxpeterxu mentioned.

Please let me know if there is anything I can do to help get this merged ASAP 🙏

dmoj/executors/NODEJS.py

pxpeterxu · 2024-01-21T19:48:53Z

@int-y1 I fixed the lint errors! Hopefully this is ready to merge now, but let me know if there are any other changes (sorry, it's been a busy few weeks!)

Xyene · 2024-01-22T04:57:19Z

Thanks, could you please rebase and squash your commits into one?

pxpeterxu · 2024-01-22T05:12:34Z

Thanks, could you please rebase and squash your commits into one?

Just done! Also, I didn't realize this project started at Don Mills C.I. -- I went there way back too! (Graduated 2010)

Xyene · 2024-01-22T05:15:08Z

Wow, small world!

codecov-commenter · 2024-01-22T15:12:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c70e803) 82.91% compared to head (7bfc8c0) 82.14%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1143      +/-   ##
==========================================
- Coverage   82.91%   82.14%   -0.78%     
==========================================
  Files         141      142       +1     
  Lines        5373     5392      +19     
==========================================
- Hits         4455     4429      -26     
- Misses        918      963      +45

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Xyene · 2024-01-22T20:36:12Z

Looks like this failed CI because of

WARNING:dmoj.cptbox:Protection fault on: 330 (sys_pkey_alloc)
WARNING:dmoj.cptbox:Arg0: 0x0000000000000000
WARNING:dmoj.cptbox:Arg1: 0x0000000000000001
WARNING:dmoj.cptbox:Arg2: 0x0000000000000001
WARNING:dmoj.cptbox:Arg3: 0x0000150daed57188
WARNING:dmoj.cptbox:Arg4: 0x0000000000000000
WARNING:dmoj.cptbox:Arg5: 0x0000000000000000

We should be able to just allow it, seems like it's part of some CFI protection in V8:

pxpeterxu · 2024-01-22T22:25:29Z

Looks like this failed CI because of

WARNING:dmoj.cptbox:Protection fault on: 330 (sys_pkey_alloc)
WARNING:dmoj.cptbox:Arg0: 0x0000000000000000
WARNING:dmoj.cptbox:Arg1: 0x0000000000000001
WARNING:dmoj.cptbox:Arg2: 0x0000000000000001
WARNING:dmoj.cptbox:Arg3: 0x0000150daed57188
WARNING:dmoj.cptbox:Arg4: 0x0000000000000000
WARNING:dmoj.cptbox:Arg5: 0x0000000000000000

We should be able to just allow it, seems like it's part of some CFI protection in V8:

* https://man7.org/linux/man-pages/man7/pkeys.7.html

* https://docs.google.com/document/d/1O2jwK4dxI3nRcOJuPYkonhTkNQfbmwdvxQMyXgeaRHo/edit#heading=h.bvaojj9fu6hc

I added a whitelist here, and it seems to be working: https://github.com/wanderlog/judge-server/actions/runs/7618060649

Force-pushed an update to this branch too

Xyene

LGTM, thanks!

yanngv29 · 2024-01-27T14:52:31Z

not sure if this is the same problem than 5 day ago but i have got error today with dmoj-autoconf:

Auto-configuring NODEJS: Protection fault on: 331 (sys_pkey_free)
Arg0: 0x0000000000000001
Arg1: 0x0000000000000001
Arg2: 0x0000000000000008
Arg3: 0xe74cfd037f6ee7e7
Arg4: 0x0000000000000007
Arg5: 0x000055555563db90

Adding 'pkey_free' to the white list do the tricks for me.

Xyene · 2024-01-28T09:35:49Z

Thanks for the report, I've put together #1164.

pxpeterxu mentioned this pull request Dec 21, 2023

Update Dockerfile to install Node.js to tier3 runtime image DMOJ/runtimes-docker#71

Merged

pxpeterxu force-pushed the master branch from 4b1eeed to 94446d3 Compare December 21, 2023 22:41

This was referenced Dec 21, 2023

Add Node.js to list of languages on README and demo languages DMOJ/online-judge#2297

Open

Update supported languages list in docs to include Node.js Javascript DMOJ/docs#155

Merged

quantum5 reviewed Dec 22, 2023

View reviewed changes

Xyene reviewed Jan 7, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

int-y1 suggested changes Jan 13, 2024

View reviewed changes

dmoj/executors/NODEJS.py Outdated Show resolved Hide resolved

dmoj/executors/NODEJS.py Outdated Show resolved Hide resolved

pxpeterxu force-pushed the master branch 2 times, most recently from 8c44618 to 7bfc8c0 Compare January 22, 2024 05:11

executors/NODEJS: add Node.js executor

1196c67

pxpeterxu force-pushed the master branch from 7bfc8c0 to 1196c67 Compare January 22, 2024 22:16

Xyene approved these changes Jan 22, 2024

View reviewed changes

Xyene merged commit f276790 into DMOJ:master Jan 22, 2024
9 checks passed

Xyene mentioned this pull request Jan 22, 2024

Add support for Node.js #1136

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Node.js executor for DMOJ #1143

Add Node.js executor for DMOJ #1143

pxpeterxu commented Dec 21, 2023 •

edited

Loading

dmoj-build commented Dec 21, 2023

quantum5 commented Dec 21, 2023

pxpeterxu commented Dec 21, 2023 •

edited

Loading

quantum5 left a comment

quantum5 Dec 22, 2023

pxpeterxu Jan 6, 2024

pxpeterxu commented Jan 6, 2024

Xyene commented Jan 7, 2024

murtadha commented Jan 13, 2024

pxpeterxu commented Jan 21, 2024

Xyene commented Jan 22, 2024

pxpeterxu commented Jan 22, 2024

Xyene commented Jan 22, 2024

codecov-commenter commented Jan 22, 2024

Xyene commented Jan 22, 2024 •

edited

Loading

pxpeterxu commented Jan 22, 2024

Xyene left a comment

yanngv29 commented Jan 27, 2024 •

edited

Loading

Xyene commented Jan 28, 2024

Add Node.js executor for DMOJ #1143

Add Node.js executor for DMOJ #1143

Conversation

pxpeterxu commented Dec 21, 2023 • edited Loading

Motivation

Changes

Related PRs on other repositories

Testing

dmoj-build commented Dec 21, 2023

quantum5 commented Dec 21, 2023

pxpeterxu commented Dec 21, 2023 • edited Loading

quantum5 left a comment

Choose a reason for hiding this comment

quantum5 Dec 22, 2023

Choose a reason for hiding this comment

pxpeterxu Jan 6, 2024

Choose a reason for hiding this comment

pxpeterxu commented Jan 6, 2024

Xyene commented Jan 7, 2024

murtadha commented Jan 13, 2024

pxpeterxu commented Jan 21, 2024

Xyene commented Jan 22, 2024

pxpeterxu commented Jan 22, 2024

Xyene commented Jan 22, 2024

codecov-commenter commented Jan 22, 2024

Codecov Report

Xyene commented Jan 22, 2024 • edited Loading

pxpeterxu commented Jan 22, 2024

Xyene left a comment

Choose a reason for hiding this comment

yanngv29 commented Jan 27, 2024 • edited Loading

Xyene commented Jan 28, 2024

pxpeterxu commented Dec 21, 2023 •

edited

Loading

pxpeterxu commented Dec 21, 2023 •

edited

Loading

Xyene commented Jan 22, 2024 •

edited

Loading

yanngv29 commented Jan 27, 2024 •

edited

Loading