Fix FI_EINTR (interrupted) corrupting internal state for no reason - #628
Fix FI_EINTR (interrupted) corrupting internal state for no reason#628jonasohland wants to merge 3 commits into
Conversation
jonasohland
added
the
backport/v1.1
KimonHoffmann
left a comment
KimonHoffmann
left a comment
There was a problem hiding this comment.
Looks good, but the handling of the new status in the demo app appears off to me.
Co-authored-by: Jonas Ohland <jonas.ohland@gmail.com> Signed-off-by: Jonas Ohland (Riedel) <jonas.ohland@riedel.net>
Co-authored-by: Jonas Ohland <jonas.ohland@gmail.com> Signed-off-by: Jonas Ohland (Riedel) <jonas.ohland@riedel.net>
c29626e to
1ba1df9
Compare
KimonHoffmann
left a comment
KimonHoffmann
left a comment
There was a problem hiding this comment.
A few subtleties regarding the handling of interruptions.
| continue; | ||
| } | ||
|
|
||
| if (status != MXL_ERR_NOT_READY && status != MXL_STATUS_OK) |
There was a problem hiding this comment.
Could use parentheses around the binary sub-expressions.
| return status; | ||
| } | ||
| } | ||
| while (status == MXL_ERR_NOT_READY && deadline > std::chrono::steady_clock::now()); |
There was a problem hiding this comment.
Could use parentheses around the binary sub-expressions.
| continue; | ||
| } | ||
|
|
||
| if (status != MXL_ERR_NOT_READY && status != MXL_STATUS_OK) |
There was a problem hiding this comment.
Could use parentheses around the binary sub-expressions.
| if (g_exit_requested) | ||
| { | ||
| return MXL_STATUS_OK; | ||
| } |
There was a problem hiding this comment.
While I understand that there is some cost to setup and teardown of this run-loop I think it'd be more appropriate to just return with MXL_ERR_INTERRUPTED and let the caller decide how to go about this.
| if (g_exit_requested) | ||
| { | ||
| return MXL_STATUS_OK; | ||
| } |
There was a problem hiding this comment.
As above wrt to forwarding the error to the caller.
|
@KimonHoffmann I am going to re-write this demo app anyway (#627), so I just want to get this fix merged with not too many other changes that will conflict with my rewrite... So maybe we can just merge this as-is? I removed the superfluous |
Co-authored-by: Jonas Ohland <jonas.ohland@gmail.com> Signed-off-by: Jonas Ohland (Riedel) <jonas.ohland@riedel.net>
4 participants
Some providers return FI_EINTR from blocking completion or event queue reads. This was (almost correctly) forwarded to the user through an exception, but leave our internal state variant in
valueless_by_exception.FI_EINTR is a recoverable error, so the user should be able to continue making progress it they choose to do so.
This PR fixes that by catching FI_EINTR early and returning it as a valid result.
Fixes #624