This is an archive of the discontinued Mercurial Phabricator instance.

sshpeer: avoid having a destructor that can block forks forever
AbandonedPublic

Authored by valentin.gatienbaron on Sep 14 2020, 11:07 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

This is solution 2. that was described in the previous commit.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

So I think I actually prefer D9019 by a hair here. @indygreg do you have a preference?

I meant for this change to be in addition to D9019, rather than an alternative, for robustness as written in a bit more detail in the commit message of D9019.

Although looking at these couple of changes again, maybe instead I could close the peers explicitly in push/pull/incoming/outgoing/id. Such a change doesn't fix all deadlocks on its own, but along with D9019, it should be enough.

Having thing explicitly close is probably better, why maybe a developer warning when this isn't the case ?

Closing the peer is what I was describing as solution 3 in D9019. But I think I was wrong in saying that it doesn't help with calls to logtoprocess in the middle of commands. I also just noticed that pull does that, in fact. I'll try and find some time to do that.

I submitted changes to close peers explicitly instead of doing this. It's definitely more invasive though.