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

Final_act can lead to program termination if the final act throws an exception #12

Closed
jbcoe opened this Issue Sep 19, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@jbcoe

jbcoe commented Sep 19, 2015

Final_act does not check that the object to be invoked is no-except invokable or try to catch any generated exceptions. This could be bad if multiple final acts are used to clear up state on scope exit.

Either Final_act should catch exceptions or there should be an requirement that the invokable object cannot throw exceptions when invoked. It may be desirable to have two implementations of Final_act:
Final_act and Final_act_noexcept which try and catch exceptions or do not depending on the noexcept status of the invokable object.

@neilmacintosh

This comment has been minimized.

Show comment
Hide comment
@neilmacintosh

neilmacintosh Sep 19, 2015

Collaborator

I agree: Final_act should check that the object being invoked is no-except. This would encourage users to think about the contexts in which it may execute.

Of course, this won't prevent program termination from occurring during Final_act's execution because the noexcept object being invoked might still end up with an exception trying to escape.

I don't like the idea of a version of Final_act catching exceptions, as I'm not sure what it would usefully do with them, and silencing them seems bad.

Collaborator

neilmacintosh commented Sep 19, 2015

I agree: Final_act should check that the object being invoked is no-except. This would encourage users to think about the contexts in which it may execute.

Of course, this won't prevent program termination from occurring during Final_act's execution because the noexcept object being invoked might still end up with an exception trying to escape.

I don't like the idea of a version of Final_act catching exceptions, as I'm not sure what it would usefully do with them, and silencing them seems bad.

@neilmacintosh neilmacintosh added the open label Sep 19, 2015

@AndrewPardoe AndrewPardoe added review and removed open labels Sep 21, 2015

@neilmacintosh neilmacintosh self-assigned this Sep 22, 2015

@jbcoe

This comment has been minimized.

Show comment
Hide comment
@jbcoe

jbcoe Sep 23, 2015

Sadly requiring the function supplied to be noexcept invokable means that one has to annotate lambdas with noexcept. If noexcept(auto) existed this problem might go away.

If a function object marked noexcept throws then that will cause program termination and can't really be the fault of Final_act.

Unless Final_acts's destructor is noexcept(false) (which is bad) I don't see what one can do with exceptions beyond swallowing them. I've seen something similar to final act (with my proposed changes) used since C++11 in a real code base without any problems.

jbcoe commented Sep 23, 2015

Sadly requiring the function supplied to be noexcept invokable means that one has to annotate lambdas with noexcept. If noexcept(auto) existed this problem might go away.

If a function object marked noexcept throws then that will cause program termination and can't really be the fault of Final_act.

Unless Final_acts's destructor is noexcept(false) (which is bad) I don't see what one can do with exceptions beyond swallowing them. I've seen something similar to final act (with my proposed changes) used since C++11 in a real code base without any problems.

@neilmacintosh

This comment has been minimized.

Show comment
Hide comment
@neilmacintosh

neilmacintosh Sep 27, 2015

Collaborator

I'm going to reject those changes, as swallowing exceptions is unhelpful to users. Final_act should be noexcept. It is conceptually just a handy way for the user to conjure up a destructor, and destructors sholud be noexcept. If something it invokes happens to throw, then the program will terminate.

Collaborator

neilmacintosh commented Sep 27, 2015

I'm going to reject those changes, as swallowing exceptions is unhelpful to users. Final_act should be noexcept. It is conceptually just a handy way for the user to conjure up a destructor, and destructors sholud be noexcept. If something it invokes happens to throw, then the program will terminate.

@neilmacintosh neilmacintosh added resolved and removed review labels Sep 27, 2015

@jbcoe

This comment has been minimized.

Show comment
Hide comment
@jbcoe

jbcoe Sep 29, 2015

I agree with your decision not to swallow exceptions but I think that the reasons given above are a bit odd.

Final_act is not a way to invoke a destructor, that's an implementation detail. Final_act is a way to ensure that something happens at scope exit (through return or exceptions being thrown). If some important cleanup function was not invoked at scope exit (it threw an exception) then program termination is probably the best idea as continuing execution in the face of failed cleanup may end badly.

jbcoe commented Sep 29, 2015

I agree with your decision not to swallow exceptions but I think that the reasons given above are a bit odd.

Final_act is not a way to invoke a destructor, that's an implementation detail. Final_act is a way to ensure that something happens at scope exit (through return or exceptions being thrown). If some important cleanup function was not invoked at scope exit (it threw an exception) then program termination is probably the best idea as continuing execution in the face of failed cleanup may end badly.

@gdr-at-ms

This comment has been minimized.

Show comment
Hide comment
@gdr-at-ms

gdr-at-ms Sep 29, 2015

Member

The key thing is that a final_act object represents a cleanup, exactly like destructor.

On Sep 29, 2015, at 6:20 AM, Jonathan B Coe <notifications@github.commailto:notifications@github.com> wrote:

I agree with your decision not to swallow exceptions but I think that the reasons given above are a bit odd.

Final_act is not a way to invoke a destructor, that's an implementation detail. Final_act is a way to ensure that something happens at scope exit (through return or exceptions being thrown). If some important cleanup function was not invoked at scope exit (it threw an exception) then program termination is probably the best idea as continuing execution in the face of failed cleanup may end badly.

Reply to this email directly or view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fMicrosoft%2fGSL%2fissues%2f12%23issuecomment-144056869&data=01%7c01%7cgdr%40microsoft.com%7c93484f448cf94722d65d08d2c8d0b44e%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=vfo89%2fC6NvaDg9RamVxiPidNdEF73woHms1f2c8CLZU%3d.

Member

gdr-at-ms commented Sep 29, 2015

The key thing is that a final_act object represents a cleanup, exactly like destructor.

On Sep 29, 2015, at 6:20 AM, Jonathan B Coe <notifications@github.commailto:notifications@github.com> wrote:

I agree with your decision not to swallow exceptions but I think that the reasons given above are a bit odd.

Final_act is not a way to invoke a destructor, that's an implementation detail. Final_act is a way to ensure that something happens at scope exit (through return or exceptions being thrown). If some important cleanup function was not invoked at scope exit (it threw an exception) then program termination is probably the best idea as continuing execution in the face of failed cleanup may end badly.

Reply to this email directly or view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fMicrosoft%2fGSL%2fissues%2f12%23issuecomment-144056869&data=01%7c01%7cgdr%40microsoft.com%7c93484f448cf94722d65d08d2c8d0b44e%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=vfo89%2fC6NvaDg9RamVxiPidNdEF73woHms1f2c8CLZU%3d.

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