Early close resource file descriptors #95
Conversation
ymmt2005
force-pushed
the
early-close-2
branch
from
d04cba0
to
2f84580
Compare
Previously, the file descriptor in a `cybozu::resource` remains open throughout its lifetime, so it was unnecessary to protect the reference to such a file descriptor. The new design is to allow the reactor to close resource file descriptors early before the resource is destructed so that yrmcdsd will be less likely to cause an out of file descriptors error. For this, we have to proctect a resource file descriptor not to be closed while it is being used. The new API design implements this with the following changes: - `resource::m_fd` is now a private member, so no direct reference is allowed. - Subclasses should use `resource::with_fd` to get the file descriptor along with a reader lock. - The reactor thread closes the file descriptor only if it can get a writer lock.
ymmt2005
force-pushed
the
early-close-2
branch
from
2f84580
to
0f52eb5
Compare
|
@nojima |
cybozu/reactor.hpp
Outdated
| if( ! valid() ) return false; | ||
| // no need to check m_closed because it becomes true only after m_valid is set to false. | ||
|
|
||
| if( std::forward<Func>(f)(m_fd) ) { |
There was a problem hiding this comment.
We don't need the perfect forwarding for f because f is not forwarded to any function, IMO. Is it sufficient to write it as f(m_fd)?
There was a problem hiding this comment.
I referenced this https://www.reddit.com/r/cpp/comments/kzvjgn/comment/gjracal/
The generated assembly looks better if we use the perfect forwarding.
But TBH, I'm not very sure...
If it does not do any harm, I'd like to keep the current way.
There was a problem hiding this comment.
I could not find a better assembly. Moreover, when using std::forward, an extra call is added.
(I checked this https://godbolt.org/z/4svKjq )
There was a problem hiding this comment.
@toshipp oops, I misread. will fix.
It generated worse code.
ymmt2005
force-pushed
the
early-close-2
branch
from
aa8f545
to
55082c3
Compare
|
Thank you for your reviews. |
Previously, the file descriptor in a
cybozu::resourceremained openthroughout its lifetime, so it was unnecessary to protect references
to such a file descriptor. However, this sometimes caused out of
file descriptors error due to its late closing.
This PR introduces a new API design around
cybozu::resourceso thatthe reactor can close file descriptors earlier than before.
The new API design consists of the following notable changes:
cybozu::resource::m_fdis now a private member, so no direct reference is allowed.cybozu::resourceholds a readers-writer lock to protectm_fd.resource::with_fdto get the file descriptor along with a reader lock.Resolve: #93