-
Notifications
You must be signed in to change notification settings - Fork 69
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
IF: Shutdown thread pool in controller destructor #2190
Conversation
Note:start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice clean up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove the member controller& self;
at line 793?
Not without the refactor of |
After discussing #2185 with @greg7mdp decided to move the shutdown of the
controller_impl
thread pool to thecontroller
destructor. The real issue here, I believe, is that we have a circular reference betweencontroller
<->controller_impl
. I think we should break that circular reference, but until we do, this seems like a much safer approach.This PR takes a step in the direction of removing
controller_impl::self
. It moves the signals tocontroller_impl
and provides accessors on controller. It also removes all internal controller uses ofself
except for passing it on totransaction_context
. Changingtransaction_context
andapply_context
to usecontroller_impl
instead ofcontroller
is a bigger change left for later. This PR does make the breaking change of signals being accessors instead of direct members oncontroller
. Since we have changed what many of the signals emit in 6.0, seems like a good time to make this interface change. The removal of usage ofself
intransaction_context
andapply_context
can happen later without impacting external users ofcontroller
.Depends on: AntelopeIO/reference-contracts#50 for the
libtester
tests.