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

Use single-argument ulimit calls #72

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

bclement-ocp
Copy link
Contributor

Some ulimit implementations (at least the one in dash, which is the /bin/sh implementation used by Debian derivatives) only accept setting one limit at a time.

$ dash
$ ulimit -t 20 -m 20
dash: 1: ulimit: too many arguments

This causes the limits set by benchpress to be ignored on these implementations when there are multiple (e.g. time and memory) limits. This patch changes the behavior to build a sequence of unrelated calls to ulimit (so ulimit -t 20 ; ulimit -m 20 ; my_prover instead of ulimit -t 20 -m 20 ; my_prover) for compatibility with these implementations.

(I had seen the ulimit: too many arguments error messages many times in prover outputs without paying too much attention to it as it seemed to still work, but yesterday we had runaway prover instances that did not stop after 18+ hours which was definite proof it didn't work (: )

Some ulimit implementations (at least the one in dash, which is the
/bin/sh implementation used by Debian derivatives) only accept setting
one limit at a time.

```shell
$ dash
$ ulimit -t 20 -m 20
dash: 1: ulimit: too many arguments
```

This causes the limits set by benchpress to be ignored on these
implementations when there are multiple (e.g. time and memory) limits.
This patch changes the behavior to build a sequence of unrelated calls
to `ulimit` (so `ulimit -t 20 ; ulimit -m 20 ; my_prover` instead of
`ulimit -t 20 -m 20 ; my_prover`) for compatibility with these
implementations.
@c-cube
Copy link
Member

c-cube commented Jul 27, 2023

Very nice, thank you. I probably ran into that at some point without realizing that ulimit is sometimes severely limited!!

@c-cube c-cube merged commit 05664d1 into sneeuwballen:master Jul 27, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants