Initialise pubsub in Windows NT service mode

i don't know what this error is about. Seen in logs many thousands of many of this log lines (650k), even if they tell to not warn about it again.

Dec 17 01:17:40.000 [warn] {HANDSHAKE} 20190 connections have failed:
Dec 17 01:17:40.000 [warn] {HANDSHAKE}  20190 connections died in state connect()ing with SSL state (No SSL object)
Dec 17 01:17:40.000 [warn] {BUG} tor_bug_occurred_(): Bug: pubsub_publish.c:37: pubsub_pub_: Non-fatal assertion !(! d) failed. (Future instances of this warning will be silenced.) (on Tor 0.4.2.5 )
Dec 17 01:17:40.000 [warn] {BUG} Bug: Tor 0.4.2.5: Non-fatal assertion !(! d) failed in pubsub_pub_ at pubsub_publish.c:37. (Stack trace not available) (on Tor 0.4.2.5 )
Dec 17 01:17:40.000 [warn] {BUG} tor_bug_occurred_(): Bug: pubsub_publish.c:37: pubsub_pub_: Non-fatal assertion !(! d) failed. (Future instances of this warning will be silenced.) (on Tor 0.4.2.5 )
Dec 17 01:17:40.000 [warn] {BUG} Bug: Tor 0.4.2.5: Non-fatal assertion !(! d) failed in pubsub_pub_ at pubsub_publish.c:37. (Stack trace not available) (on Tor 0.4.2.5 )
Dec 17 01:17:40.000 [warn] {CONTROL} Problem bootstrapping. Stuck at 0% (starting): Starting. (Connection timed out [WSAETIMEDOUT ]; TIMEOUT; count 20192; recommendation warn; host CE3FE883C6C9EF475EA097DC3E33A6F32B852DA1 at 78.129.218.56:443)

changed milestone to %Tor: 0.4.1.x-final

added 041-backport 042-backport 043-must BugSmashFund actualpoints::.2 component::core tor/tor consider-backport-after-0431 crash extra-review milestone::Tor: 0.4.1.x-final owner::nickm priority::high regression resolution::fixed reviewer::catalyst severity::normal status::closed type::defect version::tor 0.4.1.1-alpha labels

Trac:
Milestone: N/A to Tor: 0.4.2.x-final
Priority: Medium to High

Hm. What platform is this on? Do you have anything unusual in your configuration?

platform windows/mingw64

Okay. Does this happen right when you start, or after a while. Does it happen every time you try to run Tor, or only with certain configuration options?

I confirm lots of

Dec 30 12:19:49.000 [warn] tor_bug_occurred_(): Bug: pubsub_publish.c:37: pubsub_pub_: Non-fatal assertion !(! d) failed. (Future instances of this warning will be silenced.) (on Tor 0.4.2.5 )
Dec 30 12:19:49.000 [warn] Bug: Tor 0.4.2.5: Non-fatal assertion !(! d) failed in pubsub_pub_ at pubsub_publish.c:37. (Stack trace not available) (on Tor 0.4.2.5 )

lines after upgrading from 0.4.1.7 to 0.4.2.5.

That was for my relay. When I tried to run the same 0.4.2.5 binaries with clean configuration inside VirtualBox, there was no such errors. Will try to dig deeper.

Trac:
Username: Vort

Trac:
Username: Vort

notices.log

Trac:
Username: Vort

@[comment:4 nickm]: looks like problem is again related to Windows service mode of operation. I was able to reproduce the problem inside VM with clean data directory: [notices.log].

Trac:
Username: Vort

So, it happens when running as a service, but not otherwise? That might help narrow it down.

Trac:
Status: new to accepted
Owner: N/A to nickm

Yes, that was happened for me. Please recheck. If this is not the case, I will continue investigation.

Trac:
Username: Vort

as the error was reported, my tor was run as Windows service too and not otherwise yet. this is confirmable by me. But since i never saw this happening again. I sadly cannot tell more about it here.

Likely fix in branch ticket32778_041 with PR at https://github.com/torproject/tor/pull/1634 . Patch is against 0.4.1 but should merge cleanly to master.

This is a minimal fix to avoid possible stability issues. For a longer-term fix, see #32883 (moved)

Trac:
Keywords: N/A deleted, 041-backport 042-backport added
Status: accepted to needs_review
Actualpoints: N/A to .1

Trac:
Reviewer: N/A to catalyst
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.3.x-final

Replying to nickm:

Likely fix in branch ticket32778_041 with PR at https://github.com/torproject/tor/pull/1634 . Patch is against 0.4.1 but should merge cleanly to master.

This is a minimal fix to avoid possible stability issues. For a longer-term fix, see #32883 (moved) Thanks! Looks good to me, but I haven't tested it on Windows. Maybe someone else who knows how to do that could do so? If not, I'm also ok with merging this as-is because it seems like a less-common use case.

Trac:
Keywords: 041-backport 042-backport deleted, 041-backport 042-backport extra-review added

Is anyone who encountered this problem willing to verify that this patch works for them?

Trac:
Status: needs_review to needs_information

From this week's team meeting:

23:15 <+nickm> catalyst: if we can't find a tester for #32778, do you think we 
               should go ahead and merge?
23:15 -zwiebelbot:#tor-meeting- tor#32778: pubsub_pub_ - [needs_information] - 
          https://bugs.torproject.org/32778
23:16 < catalyst> nickm: yeah, it seems reasonable by inspection, and appveyor 
                  seems to be ok with it
23:16 < catalyst> i think we lack automated testing for that functionality

But it would be really good if we could get some testing on this before we ship it.

Trac:
Keywords: 041-backport 042-backport extra-review deleted, 041-backport 042-backport extra-review crash added

Trac:
Keywords: 041-backport 042-backport extra-review crash deleted, 041-backport 042-backport extra-review crash regression added

Trac:
Keywords: 041-backport 042-backport extra-review crash regression deleted, 041-backport 042-backport extra-review crash regression 043-must added

It's been about a week and no responses. I'm ok with merging this now, but am also happy to delay if people want to wait longer for feedback.

Trac:
Status: needs_information to merge_ready

There was a conflict with #32883 (moved), which replaces the pubsub calls added in this ticket, with calls to tor_run_main() (which then calls the pubsub functions). Therefore, I did an "ours" merge, to avoid taking any changes from this PR in master. (But resolve future conflicts when we backport this change.)

Unfortunately, the "ours" merge means that this code won't be tested in master.

But this PR seems reasonable to me for backport. The pattern of calls in nt_service_main() and nt_service_body() matches the pubsub call pattern used by other platforms in tor_run_main() before #32883 (moved). (And used by all platforms after #32883 (moved).)

Given the lack of testing, I'm marking it for backport after 0.4.3-stable.

Merged to master with #32705 (moved).

Trac:
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.2.x-final
Actualpoints: .1 to .2
Keywords: N/A deleted, consider-backport-after-043-stable added
Version: Tor: 0.4.2.5 to Tor: 0.4.1.1-alpha

4025cbada6ab01247275faec8dd32ae857ef0fd5 does not work at all. Looks like no one tested these fixes, sorry.

Trac:
Username: Vort

Can you be more specific, Vort? What happens when you try to use it?

Trac:
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.3.x-final
Status: merge_ready to needs_revision

When I try to launch it in service mode, process is created, but nothing happens next: no lines in log file, no ports are opened and very low RAM consumption. Looks like complete hang.

Trac:
Username: Vort

Replying to Vort:

4025cbada6ab01247275faec8dd32ae857ef0fd5 does not work at all. Looks like no one tested these fixes, sorry. Thanks for the information. That commit seems to be on master and includes the more extensive fix for #32883 (moved). Would you please try the more minimal fix at https://github.com/torproject/tor/pull/1634 ?

tor-0.4.2.5 (52d386c9) + nmathewson:ticket32778_041 (54eec534) looks good so far: no pubsub_pub errors in logs.

Trac:
Username: Vort

Replying to Vort:

tor-0.4.2.5 (52d386c9) + nmathewson:ticket32778_041 (54eec534) looks good so far: no pubsub_pub errors in logs. Thanks for the info! I've reopened #32883 (moved) so we can work on the issue in master.

Let's maybe wait a week to see if there's any more feedback on the smaller patch.

Trac:
Status: needs_revision to needs_information

Hello privacy friends! I'm the opener. I was slow to respond on ticket. Replying to catalyst:

wait a week to see if there's any more feedback on the smaller patch.

I now compiled the linked minimal patch "Initialize publish/subscribe code when running as an NT service. https://github.com/torproject/tor/pull/1634" applied against tor-0.4.2.5 running MINGW64_NT-10.0-18363 and run tor x64 as NT Service mode.

No more log message appears for me so far. Looks good. :)

This is no review in any way. Just a short feedback.

Replying to catalyst:

because it seems like a less-common use case.

Thanks for carrying anyway!

We did the master reverts and applied this patch in #32984 (moved). In this ticket, we'll track backporting the change to 0.3.5 and later.

Trac:
Status: needs_information to merge_ready
Keywords: 041-backport 042-backport extra-review crash regression 043-must deleted, 041-backport 042-backport extra-review crash regression 043-must BugSmashFund added
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.2.x-final

Trac:
Summary: pubsub_pub_ to Initialise pubsub in Windows NT service mode

Since this breaks NT services, let's backport it sooner.

Trac:
Keywords: consider-backport-after-043-stable deleted, consider-backport-after-0431 added

Merged to 0.4.1 and later.

Merged with #29819 (moved).

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.1.x-final

The: https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases/042Retrospective references mention this ticket with this "Bug Could be prevented by ... Dropping support for NT services" Dropping its feature entirely can't be the final solution? Don't drop my windows relays please. I wouldn't report this if i had knew before.

Replying to cypherpunks:

The: https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases/042Retrospective references mention this ticket with this "Bug Could be prevented by ... Dropping support for NT services" Dropping its feature entirely can't be the final solution? Don't drop my windows relays please. I wouldn't report this if i had knew before.

Don't worry, we're not planning on dropping Windows relay support any time soon.

We brainstorm lots of different ideas during our release retrospectives. If you want to see what we're actually going to do, look at the "POSSIBLE LESSONS TO TAKE FROM THIS RETROSPECTIVE" section.

In particular, we said:

Some parts of our code base are identifiably under-tested or badly held together. They include: ... Our Windows testing and QA is severely underpowered.

You can help us test our Windows code by running Windows relays, and reporting bugs. If you can, run our alpha releases, so you catch bugs early. (If people don't report bugs in a feature, then we might assume no-one is using it. That makes it more likely to be dropped.)

I didn't do this detailed analysis. I've CC'd the people who did, in case they want to comment more.

Trac:
Cc: N/A to dgoulet, nickm

closed

added 1h 36m of time spent

mentioned in issue #32883 (moved)

mentioned in issue #32984 (moved)

moved to tpo/core/tor#32778 (closed)

mentioned in issue tpo/core/tor#32883 (closed)

mentioned in issue tpo/core/tor#32984 (closed)