You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/02/19 21:50:29 UTC

[GitHub] [tvm] rkimball opened a new pull request #7480: Do not allow exceptions in destructors

rkimball opened a new pull request #7480:
URL: https://github.com/apache/tvm/pull/7480


   Exceptions raised in destructors are ignored and a warning message is displayed. Catch exceptions raised and prevent them from propagating.
   
   Raising exceptions in the ObjectBase destructor was causing stack overflow in the unit test `tests/python/unittest/test_auto_scheduler_cost_model.py` if xgboost was not installed in both windows and linux. This PR eliminates the stack overflow in that case.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] rkimball commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
rkimball commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-782464224


   There is a better solution. It is still not correct to raise exception from a destructor and we should not rely on bad code for error messages. A proper alternative is to print an error message when we catch an exception. This will give you the exact result you want. raising an exception in a destructor does not raise an exception, it just prints a message that you are trying to raise an exception where you are not supposed to.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] tqchen closed pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
tqchen closed pull request #7480:
URL: https://github.com/apache/tvm/pull/7480


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] tqchen commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-790110425


   @rkimball please rebase against the latest main.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] junrushao1994 commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-782389918


   It is exactly a bug that troubles me for years! Thanks Bob! CC: @tqchen 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] areusch commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-782462941


   I know we should not be raising exceptions from `__del__`, but we kinda went down this path and now exceptions raised in `__del__` can serve as a note to the user that they left objects live before terminating the program. in a server setting, that might be important.
   
   in this case, the underlying problem was that a Node subclass did not call super().__init__() straightaway, and then raised an exception. This caused `Object.__del__` to enter a recursive loop because `self.handle` slot was not populated. #7481 makes __del__ robust to missing handle. I prefer we merge that rather than suppress all of these error messages, as it's fairly opinionated of our library that users may not care they are leaking memory


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] areusch commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-790125836


   @tqchen I think we are not going to merge this one


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] tqchen commented on pull request #7480: Do not allow exceptions in destructors

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7480:
URL: https://github.com/apache/tvm/pull/7480#issuecomment-790154239


   Thanks @areusch for the catch. Superseded by  #7481 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org