Skip to content
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.

Sign up for GitHub

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

Jump to bottom

net: initialize socket to avoid closing random fd's #12326

Merged
merged 1 commit into from
Feb 1, 2018
Merged

net: initialize socket to avoid closing random fd's #12326

merged 1 commit into from
Feb 1, 2018
+4 −1

Conversation

theuni
Copy link
Member

@theuni theuni commented Feb 1, 2018

An excellent spot by @david60.

Even if it isn't causing the fd issue we're looking for, this should be fixed.

Sjors
Sjors reviewed Feb 1, 2018
View reviewed changes
src/net.cpp
@@ -410,7 +410,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo

// Connect
bool connected = false;
SOCKET hSocket;
SOCKET hSocket = INVALID_SOCKET;
Copy link
Member

@Sjors Sjors Feb 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why didn't a linter catch this uninitialized variable?

@maflcko maflcko added this to the 0.16.0 milestone Feb 1, 2018
@sdaftuar
Copy link
Member

sdaftuar commented Feb 1, 2018

utACK

@promag
Copy link
Member

promag commented Feb 1, 2018

utACK 62e07f0.

@Sjors
Copy link
Member

Sjors commented Feb 1, 2018

I'm currently running git bisect to figure out where the crashes started. Once that's done I'll also check if this commit makes them go away.

@laanwj
Copy link
Member

laanwj commented Feb 1, 2018

utACK 96dbd38 - this is obviously correct. Hope it solves the issue too.

laanwj pushed a commit that referenced this pull request Feb 1, 2018
@theuni @laanwj
net: initialize socket to avoid closing random fd's
e54c1ac 
Github-Pull: #12326
Rebased-From: 96dbd38
Tree-SHA512: 8b4a09974060e6d0992e7b9ec06c5de3ad2daf970c4484077fda803f37d3ed874dcb6fec226107b2aa0fa64cfe4116604ca4f90599430fcc622bbb805be55e1b
@laanwj laanwj merged commit 96dbd38 into bitcoin:master Feb 1, 2018
laanwj added a commit that referenced this pull request Feb 1, 2018
@laanwj
Merge #12326: net: initialize socket to avoid closing random fd's
84291d1 
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)

Pull request description:

  An excellent spot by @david60.

  Even if it isn't causing the fd issue we're looking for, this should be fixed.

Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
@Sjors Sjors mentioned this pull request Feb 1, 2018
Closed
@laanwj laanwj mentioned this pull request Feb 2, 2018
Closed
@dgenr8 dgenr8 mentioned this pull request Feb 6, 2018
Merged
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 16, 2018
@theuni
net: initialize socket to avoid closing random fd's
1cb1906 
Github-Pull: bitcoin#12326
Rebased-From: 96dbd38
Tree-SHA512: 8b4a09974060e6d0992e7b9ec06c5de3ad2daf970c4484077fda803f37d3ed874dcb6fec226107b2aa0fa64cfe4116604ca4f90599430fcc622bbb805be55e1b
@practicalswift practicalswift mentioned this pull request Sep 16, 2018
Closed
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 17, 2019
@lyricidal
net: initialize socket to avoid closing random fd's
dd3bc8b 
bitcoin/bitcoin#12326
"An excellent spot by @david60.

Even if it isn't causing the fd issue we're looking for, this should be fixed."
@lyricidal lyricidal mentioned this pull request Nov 17, 2019
Merged
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 24, 2019
@lyricidal
DAPS-1248 from Bitcoin: net: initialize socket to avoid closing rando…
0ffa71a 
…m fd's

bitcoin/bitcoin#12326
"An excellent spot by @david60.

Even if it isn't causing the fd issue we're looking for, this should be fixed."
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 24, 2019
@lyricidal
DAPS-1330 - Multisig [commit by commit] (v1.0.4.4-v1.0.4.6) (#973)
fd7b65e 
* DAPS-1206 - Force users to re-enter their password to reveal their mnemonic phrase

* DAPS-761 DAPS-1296 From PIVX: Unify shutdown procedure in init rather than per-app

This moves CScheduler and threadGroup to a static declaration in init
.cpp so as to avoid a potential shutdown deadlock where both are freed
before the network message handler thread has been completely released.

* DAPS-1193 Move unlock-dialog from splash screen to overview screen

* Bump version to v1.0.4.5

* reenable invalidateblock and reconsiderblock RPC

* DAPS-1248 from Bitcoin: net: initialize socket to avoid closing random fd's

bitcoin/bitcoin#12326
"An excellent spot by @david60.

Even if it isn't causing the fd issue we're looking for, this should be fixed."

* DAPS-761 From PIVX Upstream: Db runtime error cleaning the variable that needs to be logged

"Straightforward change, the variable is been cleaned before the logging. Temp variable copy to be able to print it."

Could be useful for future logging

from: PIVX-Project/PIVX#989

* DAPS-1334: Fix Runaway Exception - Move the RandAddSeedPerfmon() call into the MakeNewKey function

"CKey::MakeNewKey is responsible for generating a new private key using a cryptographic PRNG. The rest of key metadata is then generated by CWallet::GenerateNewKey.

However, the GenerateNewKey function does not call MakeNewKey before ensuring RandAddSeedPerfmon is done. If increasing the uncertainty about the state and making the PRNG output less predictable is the message to send, the step then might as well be implemented in the MakeNewKey function itself.

The initial Sanity Check, and tests are the two other consumers of the function, neither RandAddSeedPerfmon-ing. The latter out of, presumably, performance considerations. Nevertheless, the CKey access modifiers, and the ongoing libification tipped me over to log this, as it is something to consider as we move forward (in light of recent events...)."

bitcoin/bitcoin#5508
bitcoin/bitcoin#5495

* DAPS-1342 From PIVX: WIN32 Seed Cleanup: Move nLastPerfmon behind win32 ifdef.

"Code to avoid calling Perfmon too often is only needed when perfmon is actually going to get called.
This is not intended to make any functional difference in the addition of entropy to the random pool."

From PIVX-Project/PIVX#762

* DAPS-1340 From PIVX: Remove Bitcoin Core 0.8 block hardlinking

Will never be used in our case

from: PIVX-Project/PIVX#980

* use block time for transaction time

* DAPS-1333 - Windows Installer: Remove testnet shortcut

This is to eliminate some confusion for the users using the Windows Installer - a shortcut to launch Testnet was being created and added to the start menu.

* DAPS-1318 Fix for memory deallocation bug during ungraceful exit

* DAPS-1112 Dont auto-ban whitelisted nodes

* DAPS-1328 from Bitcoin: net: don't retry failed oneshot connections forever

"As introduced by (my suggestion, sorry, in) #11512, failed dns resolves end up as oneshots. But failed oneshots are re-added as oneshots, so we need to make sure that we're not queuing these up forever after failed resolves.

Rather than trying to differentiate, I think we should just not re-add failed oneshots and be done with it.

Maybe @sipa can shed a light on what the original intention was."

* Bump version to v1.0.4.6
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 26, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12326: net: initialize socket to avoid closing random fd's
74d34d1 
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)

Pull request description:

  An excellent spot by @david60.

  Even if it isn't causing the fd issue we're looking for, this should be fixed.

Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 26, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12326: net: initialize socket to avoid closing random fd's
10d269c 
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)

Pull request description:

  An excellent spot by @david60.

  Even if it isn't causing the fd issue we're looking for, this should be fixed.

Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12326: net: initialize socket to avoid closing random fd's
211c8a3 
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)

Pull request description:

  An excellent spot by @david60.

  Even if it isn't causing the fd issue we're looking for, this should be fixed.

Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 25, 2021
@laanwj
Merge bitcoin#12326: net: initialize socket to avoid closing random fd's
543ab63 
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)

Pull request description:

  An excellent spot by @david60.

  Even if it isn't causing the fd issue we're looking for, this should be fixed.

Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Sjors Sjors Sjors left review comments

Assignees
No one assigned
Labels
P2P
Projects
None yet
Milestone
0.16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@theuni @sdaftuar @promag @Sjors @laanwj @maflcko