Skip to content

Conversation

@tomoatan
Copy link
Contributor

Reverts #7596

In #7596, 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.

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;
}
}
}
t->schedule_imm(vc);

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

@randall
Copy link
Contributor

randall commented Jul 28, 2021

[approve ci]

@bneradt
Copy link
Contributor

bneradt commented Jul 28, 2021

I’m on PTO and away from my computer right now. But I see that clang analyzer fails with this. @SolidWallOfCode and I looked at the report a few months ago and that’s why we initially removed this code (to address it). We convinced ourselves that it was not called. But apparently this code can’t be removed. We’ll have to figure out how to fix the analyzer issue another way apparently.

@masaori335
Copy link
Contributor

masaori335 commented Aug 1, 2021

Unfortunately, we can't merge this PR because of clang-analyzer build failure.

#8184 has the commit from @tomoatan and the clang-analyzer fix. Thanks!

@masaori335 masaori335 closed this Aug 1, 2021
@tomoatan tomoatan deleted the revert branch August 2, 2021 01:32
@masaori335 masaori335 removed this from the 10.0.0 milestone Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants