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 system limit on file descriptors for soft limit #3641

Merged
merged 1 commit into from
Mar 7, 2019

Conversation

pdbain-ibm
Copy link
Contributor

If user tries to set the soft limit of file descriptors to unlimited on MacOS,
use the system limit instead.

Add unit test.

Fixes #3579

Signed-off-by: Peter Bain peter_bain@ca.ibm.com

@charliegracie
Copy link
Contributor

@genie-omr build all

@pdbain-ibm
Copy link
Contributor Author

I am trying to get on one of the failing machines to investigate. The test passes on a local Linux box.

@pdbain-ibm
Copy link
Contributor Author

  • Added more diagnostic output
  • Use a more conservative value for setting the file descriptor hard limit.

Tested on MacOS and Ubuntu.

@charliegracie
Copy link
Contributor

@genie-omr build all

@pdbain-ibm
Copy link
Contributor Author

The test fails if the hard and soft limits are the same finite value. Fixed.


/* lowering the hard limit is irreversible unless privileged */
if (0 != geteuid()) { /* normal user */
/* setting the hard limit from unlimited to a finite value has unpredictable results:
* the actual value may be much smaller than requested.
* In that case, just try setting it to the same value.
*/
uint64_t newHardLimit = (OMRPORT_LIMIT_UNLIMITED == rc) ? originalHardLimit: originalHardLimit - 1;
uint64_t newHardLimit = ((OMRPORT_LIMIT_UNLIMITED == rc) || (originalSoftLimit == softSetToHardLimit))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd. Can you explain this in more detail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments updated.

If user tries to set the soft limit of file descriptors to unlimited on MacOS,
use the system limit instead.

Add unit  test.  Add diagnostics output to test.

Clean up incorrect return values.

Fixes eclipse-omr#3579

Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>

Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
@charliegracie
Copy link
Contributor

@genie-omr build all

Copy link
Contributor

@charliegracie charliegracie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am good with the changes. Thanks! Over to you @youngar

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

Successfully merging this pull request may close these issues.

4 participants