Skip to content

Conversation

@tomoatan
Copy link
Contributor

Reverts #7609

In #7609, UnixNetVConnection::startEvent was removed but it's necessary in UnixNetProcessor::connect_re_internal.
In the following code, UnixNetVConnection is scheduled if some mutexes can't be locked.

if (t->is_event_type(opt->etype)) {
MUTEX_TRY_LOCK(lock, cont->mutex, t);
if (lock.is_locked()) {
MUTEX_TRY_LOCK(lock2, get_NetHandler(t)->mutex, t);
if (lock2.is_locked()) {
int ret;
ret = vc->connectUp(t, NO_FD);
if ((using_socks) && (ret == CONNECT_SUCCESS)) {
return &socksEntry->action_;
} else {
return ACTION_RESULT_DONE;
}
}
}
}
// Try to stay on the current thread if it is the right type
if (t->is_event_type(opt->etype)) {
t->schedule_imm(vc);
} else { // Otherwise, pass along to another thread of the right type
eventProcessor.schedule_imm(vc, opt->etype);
}

In this case, UnixNetVConnection::startEvent is appropriate as a handler
because UnixNetVConnection is scheduled to execute UnixNetVConnection::connectUp.
Before #7609, UnixNetVConnection::startEvent was set as a handler in constructor, so I revert it.

@masaori335
Copy link
Contributor

@tomoatan Thanks for the report. The revert seems reasonable at the first glance.
Do we have the same issue on the master branch?

@masaori335 masaori335 added this to the 8.1.2 milestone Jul 27, 2021
@masaori335
Copy link
Contributor

[approve ci]

@masaori335
Copy link
Contributor

FWIW, #7609 is the backport PR of #7596.

@tomoatan
Copy link
Contributor Author

@masaori335

Do we have the same issue on the master branch?

Yes, master branch has the same issue.

@masaori335
Copy link
Contributor

masaori335 commented Jul 28, 2021

@tomoatan Could you make another PR against the master branch? We can mark this PR as a backport PR of it when it's merged.
Because, as a convention, we fix issues on the master branch and backport it to branches.

@tomoatan
Copy link
Contributor Author

@masaori335 OK. I created PR #8182 against the master branch.

@masaori335 masaori335 requested review from ezelkow1 and removed request for SolidWallOfCode and bneradt July 28, 2021 22:42
@masaori335 masaori335 added the Backport Marked for backport for an LTS patch release label Jul 28, 2021
@masaori335 masaori335 closed this Aug 2, 2021
@masaori335 masaori335 removed this from the 8.1.2 milestone Aug 2, 2021
@masaori335
Copy link
Contributor

Close this in favor of #8195 to backport #8184 to the 8.1.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backport Marked for backport for an LTS patch release Revert

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants