Add LibC.setrlimit/LibC.getrlimit to all linux/bsd platforms

[carlhoerberg]

No description provided.

[straight-shoota]

What's the reason for this? Where are these functions used?

It appears getrlimit is not used in stdlib anymore. It was removed in #7373, but a couple of bindings are just left. I think we should just remove the leftover ones.
If it's not used in stdlib, there's no point in maintaining it. You can get access to these functions in user code by providing your own bindings.

[asterite]

Also please remember to always open an issue before a PR in order to discuss things. Or at least provide a description in the PR.

[carlhoerberg]

We use it here: https://github.com/cloudamqp/avalanchemq/blob/6560de727cdb210e4523333a0577442268c0ae45/src/stdlib/resource.cr#L74 to implement System.file_descriptor_limit and System.file_descriptor_limit=(value). Would be nice to have these platform dependent libc stuff in stdlib instead of having to deal with it in every application that happens to depend on it.

[asterite]

I think we should add a way to use getrlimit from the standard library.

Using LibC outside of the standard library is a smell.

For example Ruby has it in the Process module:

• https://ruby-doc.org/core-2.6.1/Process.html#method-c-getrlimit
• https://ruby-doc.org/core-2.6.1/Process.html#method-c-setrlimit

That way you can use it through the standard library, and we make sure it's there in all the platforms we support.

[straight-shoota]

The C bindings coming with the standard library are an implementation detail, they're not part of the public API.

There shouldn't be any C bindings that are not required for stdlib itself. But I agree with @asterite that it might be a good idea to actually add support for accessing file descriptor limits to stdlib directly.
Could just import the methods from avalanchemq.

[asterite]

That said, I wouldn't mind merging this. It doesn't hurt anymore. More so if we later plan on using those bindings.

[straight-shoota]

@carlhoerberg Would you be interested in incorporating the file_descriptor_limit and file_descriptor_limit= methods in this PR? The implementation looks good.

[carlhoerberg]

ok, can do

[carlhoerberg]

I have no idea about Windows support though.. just wrap the spec in unless flag?(:win32)?

[asterite]

If this is an int, can we add a type restriction?

[carlhoerberg]

Suggested change

	def self.file_descriptor_limit=(limit)
	def self.file_descriptor_limit=(limit : UInt32)

[straight-shoota]

UInt32 technically doesn't cut it the value is 64 bits wide on 64-bit platforms. In practice, this shouldn't be relevant, though. So we can perhaps stay with 32 bits. But maybe it should be signed instead? We try to avoid unsigned integer types in stdlib APIs.

[straight-shoota]

In any case, the same type should be applied as return type of the getter method.

[asterite]

I was thinking just Int

[straight-shoota]

For the param type restriction, that would be good. But what about the return type?

[straight-shoota]

For the specs, you can just use pending_win32 from support/win32 spec helper.

Additionally, src/crystal/system/win32/file_descriptor_limit.cr with method definitions raising NotImplementedError could already be added to let programs compile on win32.

[carlhoerberg]

Fixed your comments, settled for Int32, I don't think it's technically possible to have 2 billion FDs anyway 🤞

Regarding OS X there seems to be some issues with subprocesses and the FD limit inheritance, doesn't seem to work the same way as in Linux.. Try to dig deeper into it.