[tor-dev] [Win32] crash in test/bench.exe

[Gisle Vanem]

There is a crash in bench.exe. Running 'cdb -c g bench.exe':

ntdll!RtlpWaitOnCriticalSection+0x6b:
77a9cfd6 ff4014 inc dword ptr [eax+14h] ds:002b:00000014=????????

ChildEBP RetAddr
0137f750 77aba38a ntdll!RtlpWaitOnCriticalSection+0x6b
0137f770 77aba259 ntdll!RtlpEnterCriticalSectionContended+0x12a
*** WARNING: Unable to verify checksum for bench.exe
0137f77c 002150f7 ntdll!RtlEnterCriticalSection+0x49
0137f784 0023136c bench!tor_mutex_acquire(struct tor_mutex_t * m = 0x0039559c)+0x37
0137f794 000fc85f bench!atomic_counter_exchange(struct atomic_counter_t * counter = 0x0039559c, unsigned int newval = 6)
+0xc
(Inline) -------- bench!set_protocol_warning_severity_level+0xb
0137f7bc 0010563f bench!options_act(struct or_options_t * old_options = 0x00000000)+0x2af
0137f7d4 000f385b bench!set_options(struct or_options_t * new_val = 0x0f335730, char ** msg = 0x0137f80c)+0x6f
0137f80c 002f3707 bench!main(int argc = 0n1, char ** argv = 0x036b1e38)+0x24b
...


Seems bench.c uses some mutex which is not initialised
with 'tor_mutex_init()'. I fail to see which that should
be.

FYI.
'&m->mutex' is 0 (= eax). But adding:
tor_assert(&m->mutex);
tor_assert(&m->mutex.OwningThread);
to 'tor_mutex_acquire()' didn't help reveal the problem.

[Gisle Vanem]

Post by Gisle Vanem
Seems bench.c uses some mutex which is not initialised
with 'tor_mutex_init()'. I fail to see which that should
be.

With this patch, I no longer get that crash:

--- a/bench.c 2018-01-25 20:15:13
+++ b/bench.c 2018-03-13 12:38:09
@@ -713,6 +713,8 @@
printf("Couldn't seed RNG; exiting.\n");
return 1;
}
+
+ init_protocol_warning_severity_level();
crypto_init_siphash_key();
options = options_new();
init_logging(1);

----

But a bit crude. IMHO bench.c shouldn't have to care about such details
like mutex'es and atomic counters.

[isis agora lovecruft]

Post by Gisle Vanem

Post by Gisle Vanem
Seems bench.c uses some mutex which is not initialised
with 'tor_mutex_init()'. I fail to see which that should
be.

--- a/bench.c 2018-01-25 20:15:13
+++ b/bench.c 2018-03-13 12:38:09
@@ -713,6 +713,8 @@
printf("Couldn't seed RNG; exiting.\n");
return 1;
}
+
+ init_protocol_warning_severity_level();
crypto_init_siphash_key();
options = options_new();
init_logging(1);
----
But a bit crude. IMHO bench.c shouldn't have to care about such details
like mutex'es and atomic counters.

Hello Gisle,

Thanks for the bug report and the patch! I've made #25479 [0] for this.

Can I ask what version of Windows you were running the benchmarks on? It
seems like it might possibly have been due to differences in behaviours
between various Windows flavours in the InitializeCriticalSection() function
[1] (which was being called from tor_mutex_init(), itself called from
init_logging())?

[0]: https://trac.torproject.org/projects/tor/ticket/25479
[1]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms683472(v=vs.85).aspx)

Best regards,

[Gisle Vanem]

Post by isis agora lovecruft
Thanks for the bug report and the patch! I've made #25479 [0] for this.
Can I ask what version of Windows you were running the benchmarks on?

Windows-10 (v1701). 64-bit.

Post by isis agora lovecruft
It seems like it might possibly have been due to differences in behaviours
between various Windows flavours in the InitializeCriticalSection() function
[1] (which was being called from tor_mutex_init(), itself called from
init_logging())?

I don't think 'init_logging()' is calling 'atomic_counter_init()' with
the correct AC, but maybe an unrelated mutex. Hence the crash.

Calling 'init_protocol_warning_severity_level()' initialises the
correct (and needed) AC 'protocol_warning_severity_level'.