1664020 - ShutdownEvent::PostAndWait should return immediately if posting the event fails

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P2)

Description

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jens Stutte [:jstutte] (REO for FF 81) from bug 1633342 comment #34)

Looking at mozilla::net::ShutdownEvent::PostAndWait I see:

    rv = CacheFileIOManager::gInstance->mIOThread->Dispatch(
        this,
        CacheIOThread::WRITE);  // When writes and closing of handles is done
    MOZ_ASSERT(NS_SUCCEEDED(rv));

    TimeDuration waitTime = TimeDuration::FromSeconds(1);
    while (!mNotified) {
       ...

Shouldn't we return here if the mIOThread->Dispatch did not succeed without even entering the while loop (instead of just doing MOZ_ASSERT)?

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1664020 - ShutdownEvent::PostAndWait should return immediately if posting the event fails r=#necko

Comment 2

[Jens Stutte [:jstutte]]

Another question: am I reading TimeDuration::FromSeconds(1); right that we always block the main thread here for at least one second if the mIOThread->Dispatch was successful? That should not prevent your patch from landing, just to say...

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73bd8f2fbbf1
ShutdownEvent::PostAndWait should return immediately if posting the event fails r=necko-reviewers,kershaw

Comment 4

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jens Stutte [:jstutte] (REO for FF 81) from comment #2)

Another question: am I reading TimeDuration::FromSeconds(1); right that we always block the main thread here for at least one second if the mIOThread->Dispatch was successful? That should not prevent your patch from landing, just to say...

The monitor waits until the duration expires or until mon.Notify is called. I think that's OK.

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 73bd8f2fbbf1 (bug 1664020) for bustages complaining about CacheFileIOManager.cpp.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=build&fromchange=067329cceb6988d6a6b6541b4366c68e345c662f&tochange=941862090875537f3cbadc0fbbae6e77535fcc26&selectedTaskRun=Bpg3dXs3TzWvNzvuQHqMPw.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/941862090875537f3cbadc0fbbae6e77535fcc26

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315451909&repo=autoland&lineNumber=46752

[task 2020-09-11T13:00:51.027Z] 13:00:51     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/netwerk/cache2'
[task 2020-09-11T13:00:51.027Z] 13:00:51     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_netwerk_cache20.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/netwerk/cache2 -I/builds/worker/workspace/obj-build/netwerk/cache2 -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/cache -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Werror=implicit-function-declaration -Wno-psabi -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_netwerk_cache20.o.pp   Unified_cpp_netwerk_cache20.cpp
[task 2020-09-11T13:00:51.029Z] 13:00:51     INFO -  In file included from Unified_cpp_netwerk_cache20.cpp:38:
[task 2020-09-11T13:00:51.029Z] 13:00:51    ERROR -  /builds/worker/checkouts/gecko/netwerk/cache2/CacheFileIOManager.cpp:555:9: error: no matching function for call to 'NS_FAILED_impl'
[task 2020-09-11T13:00:51.029Z] 13:00:51     INFO -      if (NS_FAILED(rv)) {
[task 2020-09-11T13:00:51.030Z] 13:00:51     INFO -          ^~~~~~~~~~~~~
[task 2020-09-11T13:00:51.030Z] 13:00:51     INFO -  /builds/worker/workspace/obj-build/dist/include/nsError.h:32:50: note: expanded from macro 'NS_FAILED'
[task 2020-09-11T13:00:51.030Z] 13:00:51     INFO -  #define NS_FAILED(_nsresult) ((bool)MOZ_UNLIKELY(NS_FAILED_impl(_nsresult)))
[task 2020-09-11T13:00:51.031Z] 13:00:51     INFO -                                                   ^~~~~~~~~~~~~~
[task 2020-09-11T13:00:51.031Z] 13:00:51     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Likely.h:17:48: note: expanded from macro 'MOZ_UNLIKELY'
[task 2020-09-11T13:00:51.031Z] 13:00:51     INFO -  #  define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
[task 2020-09-11T13:00:51.032Z] 13:00:51     INFO -                                                 ^
[task 2020-09-11T13:00:51.032Z] 13:00:51     INFO -  /builds/worker/workspace/obj-build/dist/include/nsError.h:29:17: note: candidate function not viable: no known conversion from 'DebugOnly<nsresult>' to 'nsresult' for 1st argument
[task 2020-09-11T13:00:51.033Z] 13:00:51     INFO -  inline uint32_t NS_FAILED_impl(nsresult aErr) {
[task 2020-09-11T13:00:51.034Z] 13:00:51     INFO -                  ^
[task 2020-09-11T13:00:51.034Z] 13:00:51     INFO -  1 error generated.
[task 2020-09-11T13:00:51.035Z] 13:00:51     INFO -  /builds/worker/checkouts/gecko/config/rules.mk:723: recipe for target 'Unified_cpp_netwerk_cache20.o' failed
[task 2020-09-11T13:00:51.035Z] 13:00:51    ERROR -  make[4]: *** [Unified_cpp_netwerk_cache20.o] Error 1
[task 2020-09-11T13:00:51.036Z] 13:00:51     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/netwerk/cache2'
[task 2020-09-11T13:00:51.036Z] 13:00:51     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2020-09-11T13:00:51.037Z] 13:00:51     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/netwerk/protocol/http'
[task 2020-09-11T13:00:51.037Z] 13:00:51     INFO -  netwerk/protocol/http/Unified_cpp_protocol_http0.o
[task 2020-09-11T13:00:51.037Z] 13:00:51     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/netwerk/protocol/http'
[task 2020-09-11T13:00:54.582Z] 13:00:54     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/netwerk/cache2'
[task 2020-09-11T13:00:54.582Z] 13:00:54     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_netwerk_cache21.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/netwerk/cache2 -I/builds/worker/workspace/obj-build/netwerk/cache2 -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/cache -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Werror=implicit-function-declaration -Wno-psabi -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_netwerk_cache21.o.pp   Unified_cpp_netwerk_cache21.cpp
[task 2020-09-11T13:00:54.583Z] 13:00:54     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/netwerk/cache2'
[task 2020-09-11T13:00:54.584Z] 13:00:54     INFO -  /builds/worker/checkouts/gecko/config/recurse.mk:72: recipe for target 'netwerk/cache2/target-objects' failed
[task 2020-09-11T13:00:54.584Z] 13:00:54    ERROR -  make[3]: *** [netwerk/cache2/target-objects] Error 2
[task 2020-09-11T13:00:54.584Z] 13:00:54     INFO -  make[3]: *** Waiting for unfinished jobs....

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a1f38a439e6
ShutdownEvent::PostAndWait should return immediately if posting the event fails r=necko-reviewers,kershaw

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7a1f38a439e6