This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
handle exit and avoid threads hanging #137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arkpar
reviewed
Apr 16, 2018
polkadot/consensus/src/service.rs
Outdated
| @@ -265,9 +265,11 @@ impl Service { | |||
| client: Arc<C>, | |||
| network: Arc<net::ConsensusService>, | |||
| transaction_pool: Arc<Mutex<TransactionPool>>, | |||
| key: ed25519::Pair | |||
| key: ed25519::Pair, | |||
| exit: ::exit_future::Exit, | |||
There was a problem hiding this comment.
As I've mentioned in personal discussion I can't say I like this. This just introduced additional complexity to the service interface and leaks away implementation details. What happens if the service is dropped before exit is signalled? This is something the user of this interface should not worry about.
arkpar
reviewed
Apr 16, 2018
polkadot/service/src/lib.rs
Outdated
| // before returning, make sure the network is started. avoids a race | ||
| // between the drop killing notification listeners and the new notification | ||
| // stream being started. | ||
| barrier.wait(); |
There was a problem hiding this comment.
Is this really necessary? There's no event loop blocked on the network anymore.
There was a problem hiding this comment.
it's also nice to make sure the network is started before we begin the consensus process.
gavofyork
reviewed
Apr 18, 2018
|
|
||
| core.handle().spawn(events); | ||
| if let Err(e) = core.run(exit) { | ||
| debug!("Polkadot service event loop shutdown with {:?}", e); |
There was a problem hiding this comment.
not needed fro this PR, but in general i'd like to get away from anonymous messages at anything more verbose than the info! level. enabling debug and trace is a currently a mess, so doesn't really make sense to enable them without specifying a target filter.
There was a problem hiding this comment.
You can always filter by module. I usually use an explicit target to logically group logs coming from several modules under a single filter. If there is just a single module there's no need for that.
gavofyork
approved these changes
Apr 18, 2018
gavofyork
added
A7-looksgoodcantmerge
and removed
A0-please_review
|
|
rphmeier
added
A0-please_review
arkpar
added
A8-looksgood
and removed
A0-please_review
lamafab
pushed a commit
to lamafab/substrate
that referenced
this pull request
Jun 16, 2020
liuchengxu
pushed a commit
to chainx-org/substrate
that referenced
this pull request
Aug 23, 2021
* btcbridge CandidateTx and records withdraw cache * Fix precision and initialization error * Initialize balance of alice same with activation_per_share * Tweak initial intention profile * fix bug for withdraw in canonize, add no_withdrawal flag * Can not unstake when still in frozen * Fix some bug * Tweak session_length and sessions_per_era * Fix bug: Only candidate confirmed can create new proposal * Fix unexpect deposit * Tweak staking fees * Fix bug: Add unexpect in Candidate to handle unexpect deposit * Fix/match precision (paritytech#131) * fix pending order precision * add tests * Update genesis_config * Fix when candidate initialize * Recover AccountMap to support btc register * Update genesis BlockHeader * Fix select utxo must balance > 0 * move best index set before deposit/withdraw in canonize * Fix build error * Modify > irr_block as >= * Fix bug: Only candidate is not confirmed can modifi it status * Fix btc transaction correlation * Adjust PCX precision in session reward (paritytech#134) * Fix/match precision (paritytech#132) * fix pending order precision * add tests * add reserve last * 1. Fix UTXOList bug (paritytech#136) 2. Update genesis_config irr_block from 0 to 2 * Remove String (paritytech#137) * Reserve initial nomination (paritytech#138) * Fix wasm build error * Init nominees of initial intenions (paritytech#139) * UTXO only store value > 0 * Init channel (paritytech#140) * Init channel relationship * Init genesis intention * chance channel name (paritytech#141) * fix fill fee (paritytech#142) * Tweak parameters
liuchengxu
added a commit
to chainx-org/substrate
that referenced
this pull request
Aug 23, 2021
* Add new_era() in election * Intergate new_session() * Add reward proposal09 * Add TreasuryAccount trait to get accound id of Treasury Module in Staking * SimpleTreasuryAccount * Start rewarding test * Impl proposal09 reward distribution * Impl asset mining reward * Start session management test in staking * Nits * Fix tests * Add pallet-session into runtime * //blockauthor? * .
helin6
pushed a commit
to boolnetwork/substrate
that referenced
this pull request
Jul 25, 2023
* Document the `Call` derive macro * Obey the fmt * Update proc-macro/src/lib.rs Co-authored-by: Andrew Jones <ascjones@gmail.com> * Review feedback Co-authored-by: Andrew Jones <ascjones@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.