Opened 6 weeks ago

Last modified 5 weeks ago

#32883 needs_revision defect

Use tor_api.h entry points for ntmain.c

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: BugSmashFund
Cc: catalyst, Vort Actual Points:
Parent ID: Points: .2
Reviewer: ahf Sponsor:

Description

ntmain.c currently launches tor by calling several functions in main.c directly, but this has led to the two getting out of sync. (See #32778 for one example.)

Instead, ntmain.c should use the functions in tor_api.h to launch Tor.

Child Tickets

TicketStatusOwnerSummaryComponent
#32984closednickmRevert #32883 for now and apply #32778 (so nt services can work in 0.4.3)Core Tor/Tor

Change History (12)

comment:1 Changed 6 weeks ago by nickm

comment:2 Changed 6 weeks ago by nickm

(see #23081 for another bug caused by this duplicated code)

Edit: fix ticket number typo

Last edited 5 weeks ago by teor (previous) (diff)

comment:3 Changed 6 weeks ago by nickm

Status: assignedneeds_review

comment:4 Changed 5 weeks ago by dgoulet

Reviewer: ahf

comment:5 Changed 5 weeks ago by ahf

Status: needs_reviewmerge_ready

Looks good.

comment:6 Changed 5 weeks ago by nickm

merged!

comment:7 Changed 5 weeks ago by nickm

Resolution: implemented
Status: merge_readyclosed

comment:8 Changed 5 weeks ago by catalyst

Cc: catalyst Vort added
Resolution: implemented
Status: closedreopened

Based on ticket:32778#comment:22 and ticket:32778#comment:24, it looks like the NT service stuff doesn't work at all (stalls, no output, etc) after these changes, while the more minimal fix in #32778 does.

comment:9 Changed 5 weeks ago by nickm

Maybe, given our release timeframe, we should consider reverting this in 0.4.3, and investigating more closely in 0.4.4?

comment:10 in reply to:  9 Changed 5 weeks ago by teor

Status: reopenedneeds_revision

Replying to nickm:

Maybe, given our release timeframe, we should consider reverting this in 0.4.3, and investigating more closely in 0.4.4?

Let's revert this change.

The revert might be complicated. I did an "ours" merge of #32778 to avoid conflicts with the code in this ticket, which we now want to revert. Let's make sure that #32778 ends up in master for 0.4.3, and this code does not.

Let's make a new PR that reverts PR 1635, and merges PR 1634.

comment:11 Changed 5 weeks ago by nickm

I've made a child ticket to revert the change here. We can try to figure out more about making this approach work in 0.4.4

comment:12 Changed 5 weeks ago by teor

Keywords: BugSmashFund added
Milestone: Tor: 0.4.3.x-finalTor: 0.4.4.x-final
Note: See TracTickets for help on using tickets.