Open Bug 1579859 Opened 5 years ago Updated 2 years ago

jar logging tries to access the directory service from non-mainthread

Categories

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

defect

Tracking

()

Tracking Status
firefox71 --- affected

People

(Reporter: Gijs, Unassigned, NeedInfo)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(1 obsolete file)

I believe this is the last thing blocking landing bug 1163021.

The trypush at https://treeherder.mozilla.org/#/jobs?repo=try&revision=604b8f3ee872f8842fb6c472050d54303149216b indicates that the run step of 3-stage PGO profiling still crashes if we add a release assert guarding against non-mainthread access of the directory service.

Running this step locally, it seems this is due to the MOZ_JAR_LOG_FILE environment variable - you can easily reproduce with a local build with the patch from bug 1163021 if you set the MOZ_JAR_LOG_FILE env var. It yields the following stack:

0x00007fffe7394752 in nsDirectoryService::Get(char const*, nsID const&, void**) () from path/to/firefox/libxul.so
(gdb) bt
#0  0x00007fffe7394752 in nsDirectoryService::Get(char const*, nsID const&, void**) ()
    at path/to/firefox/libxul.so
#1  0x00007fffe83002c6 in nsZipArchive::OpenArchive(nsZipHandle*, PRFileDesc*) () at path/to/firefox/libxul.so
#2  0x00007fffe82ffc4d in nsJAR::Open(nsIFile*) () at path/to/firefox/libxul.so
#3  0x00007fffe8303f99 in nsZipReaderCache::GetZip(nsIFile*, nsIZipReader**, bool) ()
    at path/to/firefox/libxul.so
#4  0x00007fffe83070ed in CreateLocalJarInput(nsIZipReaderCache*, nsIFile*, nsTSubstring<char> const&, nsIJARURI*, nsTSubstring<char> const&, nsJARInputThunk**) () at path/to/firefox/libxul.so
#5  0x00007fffe8313f67 in mozilla::detail::RunnableFunction<nsJARChannel::OpenLocalFile()::$_0>::Run() ()
    at path/to/firefox/libxul.so
#6  0x00007fffe740aaa7 in nsThreadPool::Run() () at path/to/firefox/libxul.so
#7  0x00007fffe740ae75 in non-virtual thunk to nsThreadPool::Run() () at path/to/firefox/libxul.so
#8  0x00007fffe7405381 in nsThread::ProcessNextEvent(bool, bool*) () at path/to/firefox/libxul.so
#9  0x00007fffe7408589 in NS_ProcessNextEvent(nsIThread*, bool) () at path/to/firefox/libxul.so
#10 0x00007fffe7cd4390 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ()
    at path/to/firefox/libxul.so
#11 0x00007fffe7c2d4b6 in MessageLoop::Run() () at path/to/firefox/libxul.so
#12 0x00007fffe7401ef5 in nsThread::ThreadFunc(void*) () at path/to/firefox/libxul.so
#13 0x00007ffff6f8c302 in _pt_root () at path/to/firefox/libnspr4.so
#14 0x00007ffff7f8f182 in start_thread (arg=<optimised out>) at pthread_create.c:486
#15 0x00007ffff7b5bb1f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

So this is tripped by asyncopen on a jar channel hitting the codepath at https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/modules/libjar/nsZipArchive.cpp#353-384 .

This should be straightforward to fix.

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6a5d823c5ae4 don't use nsDirectoryService::Get to get gre dir on a non-main thread, r=froydnj

Backed out changeset 6a5d823c5ae4 (Bug 1579859) for causing build bustages

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=6a5d823c5ae4baa5aae89d197915f1dfaf7d3c61&tochange=4a36a66e34128f517a7e5ba479b2d8849c4b9c5a&selectedJob=265772562

Backout link: https://hg.mozilla.org/integration/autoland/rev/4a36a66e34128f517a7e5ba479b2d8849c4b9c5a

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

[task 2019-09-09T19:58:09.541Z] 19:58:09 INFO - buildsymbols> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.983Z] 19:58:09 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.983Z] 19:58:09 INFO - package-tests> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.process_install_manifest --track install__test_files.track _tests _build_manifests/install/_test_files
[task 2019-09-09T19:58:09.983Z] 19:58:09 INFO - package-tests> Elapsed: 12.11s; From _tests: Kept 18 existing; Added/updated 36741; Removed 0 files and 0 directories.
[task 2019-09-09T19:58:09.983Z] 19:58:09 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> rm -rf dist/test-stage
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/bin
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/bin/components
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/certs
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/config
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/modules
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/tools/mach
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.991Z] 19:58:09 INFO - package-tests> /usr/bin/make -C ./js/src/tests stage-package
[task 2019-09-09T19:58:09.992Z] 19:58:09 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:09.992Z] 19:58:09 INFO - package-tests> ./config/nsinstall -D dist/test-stage/extensions/
[task 2019-09-09T19:58:09.992Z] 19:58:09 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> cp /builds/worker/workspace/build/src/testing/tools/mach_test_package_bootstrap.py dist/test-stage/tools/mach_bootstrap.py
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> cp /builds/worker/workspace/build/src/mach dist/test-stage
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> ./config/nsinstall -D dist/test-stage/steeplechase/
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> cp -RL ./_tests/steeplechase dist/test-stage/steeplechase/tests
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> cp -RL dist/xpi-stage/specialpowers dist/test-stage/steeplechase
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> cp -RL /builds/worker/workspace/build/src/testing/profiles/common/user.js dist/test-stage/steeplechase/prefs_general.js
[task 2019-09-09T19:58:10.007Z] 19:58:10 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.043Z] 19:58:10 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.043Z] 19:58:10 INFO - package-tests> ./config/nsinstall -D dist/test-stage/config
[task 2019-09-09T19:58:10.043Z] 19:58:10 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.171Z] 19:58:10 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:10.171Z] 19:58:10 INFO - package-tests> ./config/nsinstall -D dist/test-stage/cppunittest
[task 2019-09-09T19:58:10.187Z] 19:58:10 INFO - package-tests> /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestBaseProfiler dist/test-stage/cppunittest/TestBaseProfiler; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestScopeExit dist/test-stage/cppunittest/TestScopeExit; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestMaybe dist/test-stage/cppunittest/TestMaybe; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestNotNull dist/test-stage/cppunittest/TestNotNull; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestEndian dist/test-stage/cppunittest/TestEndian; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestBinarySearch dist/test-stage/cppunittest/TestBinarySearch; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestLinkedList dist/test-stage/cppunittest/TestLinkedList; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestTemplateLib dist/test-stage/cppunittest/TestTemplateLib; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestTuple dist/test-stage/cppunittest/TestTuple; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestCountPopulation dist/test-stage/cppunittest/TestCountPopulation; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestThreadSafeWeakPtr dist/test-stage/cppunittest/TestThreadSafeWeakPtr; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSegmentedVector dist/test-stage/cppunittest/TestSegmentedVector; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSPSCQueue dist/test-stage/cppunittest/TestSPSCQueue; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestArray dist/test-stage/cppunittest/TestArray; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestDoublyLinkedList dist/test-stage/cppunittest/TestDoublyLinkedList; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestNonDereferenceable dist/test-stage/cppunittest/TestNonDereferenceable; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestDefineEnum dist/test-stage/cppunittest/TestDefineEnum; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestXorShift128PlusRNG dist/test-stage/cppunittest/TestXorShift128PlusRNG; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestUtf8 dist/test-stage/cppunittest/TestUtf8; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestPoisonArea dist/test-stage/cppunittest/TestPoisonArea; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestIntegerRange dist/test-stage/cppunittest/TestIntegerRange; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestTypeTraits dist/test-stage/cppunittest/TestTypeTraits; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestAlgorithm dist/test-stage/cppunittest/TestAlgorithm; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestRandomNum dist/test-stage/cppunittest/TestRandomNum; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestIntegerPrintfMacros dist/test-stage/cppunittest/TestIntegerPrintfMacros; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestVariant dist/test-stage/cppunittest/TestVariant; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestPrintf dist/test-stage/cppunittest/TestPrintf; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestRefPtr dist/test-stage/cppunittest/TestRefPtr; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestMathAlgorithms dist/test-stage/cppunittest/TestMathAlgorithms; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSHA1 dist/test-stage/cppunittest/TestSHA1; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSplayTree dist/test-stage/cppunittest/TestSplayTree; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestEnumeratedArray dist/test-stage/cppunittest/TestEnumeratedArray; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestCeilingFloor dist/test-stage/cppunittest/TestCeilingFloor; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/ShowSSEConfig dist/test-stage/cppunittest/ShowSSEConfig; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestUniquePtr dist/test-stage/cppunittest/TestUniquePtr; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestMacroForEach dist/test-stage/cppunittest/TestMacroForEach; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestTextUtils dist/test-stage/cppunittest/TestTextUtils; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestFunctionTypeTraits dist/test-stage/cppunittest/TestFunctionTypeTraits; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSaturate dist/test-stage/cppunittest/TestSaturate; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestArrayUtils dist/test-stage/cppunittest/TestArrayUtils; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestWeakPtr dist/test-stage/cppunittest/TestWeakPtr; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestBloomFilter dist/test-stage/cppunittest/TestBloomFilter; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestSmallPointerArray dist/test-stage/cppunittest/TestSmallPointerArray; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestEnumSet dist/test-stage/cppunittest/TestEnumSet; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestTypedEnum dist/test-stage/cppunittest/TestTypedEnum; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestCheckedInt dist/test-stage/cppunittest/TestCheckedInt; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestAtomics dist/test-stage/cppunittest/TestAtomics; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestVector dist/test-stage/cppunittest/TestVector; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestCasting dist/test-stage/cppunittest/TestCasting; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestJSONWriter dist/test-stage/cppunittest/TestJSONWriter; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestResult dist/test-stage/cppunittest/TestResult; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestMacroArgs dist/test-stage/cppunittest/TestMacroArgs; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestRange dist/test-stage/cppunittest/TestRange; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestFastBernoulliTrial dist/test-stage/cppunittest/TestFastBernoulliTrial; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestWrappingOperations dist/test-stage/cppunittest/TestWrappingOperations; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestBufferList dist/test-stage/cppunittest/TestBufferList; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestEnumTypeTraits dist/test-stage/cppunittest/TestEnumTypeTraits; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestCountZeroes dist/test-stage/cppunittest/TestCountZeroes; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestPair dist/test-stage/cppunittest/TestPair; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestFloatingPoint dist/test-stage/cppunittest/TestFloatingPoint; /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/cppunittests/TestRollingMean dist/test-stage/cppunittest/TestRollingMean;
[task 2019-09-09T19:58:10.187Z] 19:58:10 INFO - package-tests> /builds/worker/fetches/binutils/bin/objcopy --strip-debug dist/bin/jsapi-tests dist/test-stage/cppunittest/jsapi-tests
[task 2019-09-09T19:58:10.187Z] 19:58:10 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:12.491Z] 19:58:12 INFO - package> make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/browser/installer'
[task 2019-09-09T19:58:12.491Z] 19:58:12 INFO - package> NO_PKG_FILES="core bsdecho js js-config jscpucfg nsinstall viewer TestGtkEmbed elf-dynstr-gc mangle* maptsv* mfc* msdump* msmap* nm2tsv* nsinstall* res/samples res/throbber shlibsign* certutil* pk12util* BadCertAndPinningServer* OCSPStaplingServer* SanctionsTestServer* GenerateOCSPResponse* chrome/chrome.rdf chrome/app-chrome.manifest chrome/overlayinfo components/compreg.dat components/xpti.dat content_unit_tests necko_unit_tests *.dSYM SmokeDMD"
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.py -DPKG_LOCALE_MANIFEST=/builds/worker/workspace/build/src/obj-firefox/browser/installer/locale-manifest.in -DMOZ_APP_NAME=firefox -DPREF_DIR=defaults/preferences -DMOZ_GTK=1 -DJAREXT= -DMOZ_ENABLE_SKIA_PDF=1 -DMOZ_CHILD_PROCESS_NAME=plugin-container -DNECKO_WIFI -DDLL_PREFIX=lib -DDLL_SUFFIX=.so -DBIN_SUFFIX= -DDIR_MACOS= -DDIR_RESOURCES= -DBINPATH='bin' -DRESPATH='bin' -DLPROJ_ROOT=en -DCLANG_CXX -DENABLE_MARIONETTE=1 -DACCESSIBILITY=1 -DATK_MAJOR_VERSION=2 -DATK_MINOR_VERSION=8 -DATK_REV_VERSION=0 -DBROWSER_CHROME_URL=chrome://browser/content/browser.xhtml -DBROWSER_CHROME_URL_QUOTED='"chrome://browser/content/browser.xhtml"' -DBUILD_CTYPES=1 -DCROSS_COMPILE='' -DEARLY_BETA_OR_EARLIER=1 -DENABLE_INTL_API=1 -DENABLE_REMOTE_AGENT=1 -DENABLE_SYSTEM_EXTENSION_DIRS=1 -DENABLE_TESTS=1 -DENABLE_TYPED_OBJECTS=1 -DENABLE_WASM_BIGINT=1 -DENABLE_WASM_BULKMEM_OPS=1 -DENABLE_WASM_CRANELIFT=1 -DENABLE_WASM_GC=1 -DENABLE_WASM_REFTYPES=1 -DFORCE_PR_LOG=1 -DFUNCPROTO=15 -DGDK_VERSION_MAX_ALLOWED=GDK_VERSION_3_4 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_32 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 -DGL_PROVIDER_EGL=1 -DGTEST_HAS_RTTI=0 -DHAVE_64BIT_BUILD=1 -DHAVE_ALLOCA_H=1 -DHAVE_BYTESWAP_H=1 -DHAVE_CLOCK_MONOTONIC=1 -DHAVE_CPUID_H=1 -DHAVE_DIRENT_H=1 -DHAVE_DLADDR=1 -DHAVE_DLOPEN=1 -DHAVE_FONTCONFIG_FCFREETYPE_H=1 -DHAVE_FT_BITMAP_SIZE_Y_PPEM=1 -DHAVE_FT_GLYPHSLOT_EMBOLDEN=1 -DHAVE_FT_LOAD_SFNT_TABLE=1 -DHAVE_GETOPT_H=1 -DHAVE_GMTIME_R=1 -DHAVE_INTTYPES_H=1 -DHAVE_LCHOWN=1 -DHAVE_LINUX_IF_ADDR_H=1 -DHAVE_LINUX_PERF_EVENT_H=1 -DHAVE_LINUX_QUOTA_H=1 -DHAVE_LINUX_RTNETLINK_H=1 -DHAVE_LOCALECONV=1 -DHAVE_LOCALTIME_R=1 -DHAVE_LSTAT64=1 -DHAVE_MALLINFO=1 -DHAVE_MALLOC_H=1 -DHAVE_MALLOC_USABLE_SIZE=1 -DHAVE_MEMALIGN=1 -DHAVE_MEMMEM=1 -DHAVE_NETINET_IN_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_POSIX_FADVISE=1 -DHAVE_POSIX_FALLOCATE=1 -DHAVE_POSIX_MEMALIGN=1 -DHAVE_PTHREAD_H=1 -DHAVE_RES_NINIT=1 -DHAVE_SETPRIORITY=1 -DHAVE_STAT64=1 -DHAVE_STDINT_H=1 -DHAVE_STRERROR=1 -DHAVE_STRNDUP=1 -DHAVE_SYSCALL=1 -DHAVE_SYS_QUEUE_H=1 -DHAVE_SYS_QUOTA_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_THREAD_TLS_KEYWORD=1 -DHAVE_TRUNCATE64=1 -DHAVE_UNISTD_H=1 -DHAVE_VALLOC=1 -DHAVE_VA_COPY=1 -DHAVE_VA_LIST_AS_ARRAY=1 -DHAVE_VISIBILITY_ATTRIBUTE=1 -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1 -DHAVE__UNWIND_BACKTRACE=1 -DHAVE___CXA_DEMANGLE=1 -DJS_64BIT=1 -DJS_BUILD_BINAST=1 -DJS_CODEGEN_X64=1 -DJS_DEFAULT_JITREPORT_GRANULARITY=3 -DJS_PUNBOX64=1 -DJS_TRACE_LOGGING=1 -DMALLOC_H='<malloc.h>' -DMALLOC_USABLE_SIZE_CONST_PTR='' -DMOZILLA_OFFICIAL=1 -DMOZILLA_UAVERSION='"71.0"' -DMOZILLA_VERSION='"71.0a1"' -DMOZILLA_VERSION_U=71.0a1 -DMOZ_ACCESSIBILITY_ATK=1 -DMOZ_ALLOW_LEGACY_EXTENSIONS=1 -DMOZ_APP_UA_NAME='""' -DMOZ_APP_UA_VERSION='"71.0a1"' -DMOZ_AV1=1 -DMOZ_BLOCK_PROFILE_DOWNGRADE=1 -DMOZ_BUILD_APP=browser -DMOZ_BUNDLED_FONTS=1 -DMOZ_CLANG_PLUGIN=1 -DMOZ_CRASHREPORTER=1 -DMOZ_DATA_REPORTING=1 -DMOZ_DAV1D_ASM=1 -DMOZ_DEDICATED_PROFILES=1 -DMOZ_DEMANGLE_SYMBOLS=1 -DMOZ_DISTRIBUTION_ID='"org.mozilla"' -DMOZ_DLL_PREFIX='"lib"' -DMOZ_DLL_SUFFIX='".so"' -DMOZ_DMD=1 -DMOZ_ENABLE_DBUS=1 -DMOZ_ENABLE_SKIA=1 -DMOZ_ENABLE_SKIA_PDF=1 -DMOZ_ENABLE_SKIA_PDF_SFNTLY=1 -DMOZ_FFMPEG=1 -DMOZ_FFVPX=1 -DMOZ_FMP4=1 -DMOZ_GECKO_PROFILER=1 -DMOZ_GECKO_PROFILER_PARSE_ELF=1 -DMOZ_GLUE_IN_PROGRAM=1 -DMOZ_HAS_REMOTE=1 -DMOZ_INSTRUMENT_EVENT_LOOP=1 -DMOZ_LIBAV_FFT=1 -DMOZ_LOGGING=1 -DMOZ_LTO=1 -DMOZ_MACBUNDLE_ID=org.mozilla.nightly -DMOZ_MEMORY=1 -DMOZ_NEW_CERT_STORAGE=1 -DMOZ_NEW_NOTIFICATION_STORE=1 -DMOZ_NEW_XULSTORE=1 -DMOZ_NORMANDY=1 -DMOZ_PEERCONNECTION=1 -DMOZ_PHOENIX=1 -DMOZ_PLACES=1 -DMOZ_PROFILER_MEMORY=1 -DMOZ_PROFILING=1 -DMOZ_PULSEAUDIO=1 -DMOZ_RAW=1 -DMOZ_REPLACE_MALLOC=1 -DMOZ_RUST_SIMD=1 -DMOZ_SAMPLE_TYPE_FLOAT32=1 -DMOZ_SANDBOX=1 -DMOZ_SCTP=1 -DMOZ_SERVICES_HEALTHREPORT=1 -DMOZ_SRTP=1 -DMOZ_STATIC_JS=1 -DMOZ_TELEMETRY_ON_BY_DEFAULT=1 -DMOZ_TELEMETRY_REPORTING=1 -DMOZ_TREE_CAIRO=1 -DMOZ_TREE_PIXMAN=1 -DMOZ_UPDATER=1 -DMOZ_UPDATE_CHANNEL=nightly-autoland -DMOZ_USER_DIR='".mozilla"' -DMOZ_VERIFY_MAR_SIGNATURE=1 -DMOZ_VORBIS=1 -DMOZ_VTUNE=1 -DMOZ_WAYLAND=1 -DMOZ_WEBM_ENCODER=1 -DMOZ_WEBRTC=1 -DMOZ_WEBRTC_ASSERT_ALWAYS=1 -DMOZ_WEBRTC_SIGNALING=1 -DMOZ_WEBSPEECH=1 -DMOZ_WEBSPEECH_TEST_BACKEND=1 -DMOZ_WIDGET_GTK=1 -DMOZ_X11=1 -DMOZ_XUL=1 -DNIGHTLY_BUILD=1 -DNO_NSPR_10_SUPPORT=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DSTATIC_JS_API=1 -DSTDC_HEADERS=1 -DTARGET_XPCOM_ABI='"x86_64-gcc3"' -DUSE_SKIA=1 -DU_STATIC_IMPLEMENTATION=1 -DU_USING_ICU_NAMESPACE=0 -DVA_COPY=va_copy -DWASM_PRIVATE_REFTYPES=1 -DXP_LINUX=1 -DXP_UNIX=1 -D_REENTRANT=1 -DAB_CD=en-US
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package> --format omni
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package> --removals /builds/worker/workspace/build/src/browser/installer/removed-files.in
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package>
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package>
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package> --minify
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package>
[task 2019-09-09T19:58:12.493Z] 19:58:12 INFO - package> --jarlog /builds/worker/fetches/en-US.log
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> --compress none
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/browser/installer/package-manifest.in '../../dist' '../../dist'/firefox
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package>
[task 2019-09-09T19:58:12.494Z] 19:58:12 ERROR - package> Traceback (most recent call last):
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> File "/builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.py", line 339, in <module>
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> main()
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> File "/builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.py", line 331, in main
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> raise Exception('No jar log data for %s' % p)
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> Exception: No jar log data for browser/omni.ja
[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:25: recipe for target 'stage-package' failed
[task 2019-09-09T19:58:12.495Z] 19:58:12 ERROR - package> make[5]: *** [stage-package] Error 1
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - package> make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/browser/installer'
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:108: recipe for target 'make-package' failed
[task 2019-09-09T19:58:12.495Z] 19:58:12 ERROR - package> make[4]: *** [make-package] Error 2
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/config/rules.mk:389: recipe for target 'default' failed
[task 2019-09-09T19:58:12.495Z] 19:58:12 ERROR - package> make[3]: *** [default] Error 2
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - package> /builds/worker/workspace/build/src/browser/build.mk:6: recipe for target 'package' failed
[task 2019-09-09T19:58:12.495Z] 19:58:12 ERROR - package> make[2]: *** [package] Error 2
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - /builds/worker/workspace/build/src/build/moz-automation.mk:112: recipe for target 'automation/package' failed
[task 2019-09-09T19:58:12.495Z] 19:58:12 ERROR - make[1]: *** [automation/package] Error 2
[task 2019-09-09T19:58:12.495Z] 19:58:12 INFO - make[1]: *** Waiting for unfinished jobs....
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> ./config/nsinstall -D dist/test-stage/gtest/gtest_bin/gtest
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> LIBXUL_BASE=tail -1 dist/bin/dependentlibs.list &&
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> /builds/worker/fetches/binutils/bin/objcopy --strip-debug
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> dist/bin/gtest/$LIBXUL_BASE dist/test-stage/gtest/gtest_bin/gtest/$LIBXUL_BASE
[task 2019-09-09T19:58:12.716Z] 19:58:12 INFO - package-tests> cp -RL ./_tests/gtest dist/test-stage
[task 2019-09-09T19:58:12.717Z] 19:58:12 INFO - package-tests> cp /builds/worker/workspace/build/src/testing/gtest/rungtests.py dist/test-stage/gtest
[task 2019-09-09T19:58:12.717Z] 19:58:12 INFO - package-tests> cp /builds/worker/workspace/build/src/testing/gtest/remotegtests.py dist/test-stage/gtest
[task 2019-09-09T19:58:12.717Z] 19:58:12 INFO - package-tests> cp dist/bin/dependentlibs.list.gtest dist/test-stage/gtest
[task 2019-09-09T19:58:12.717Z] 19:58:12 INFO - package-tests> cp ./mozinfo.json dist/test-stage/gtest
[task 2019-09-09T19:58:12.717Z] 19:58:12 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:13.206Z] 19:58:13 INFO - package-generated-sources> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:13.206Z] 19:58:13 INFO - package-generated-sources> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.package_generated_sources 'dist/target.generated-files.tar.gz'
[task 2019-09-09T19:58:13.206Z] 19:58:13 INFO - package-generated-sources> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:21.362Z] 19:58:21 INFO - package-tests> make[3]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/js/src/tests'
[task 2019-09-09T19:58:21.362Z] 19:58:21 INFO - package-tests> ../../../config/nsinstall -D ../../../dist/test-stage/jsreftest/tests
[task 2019-09-09T19:58:21.362Z] 19:58:21 INFO - package-tests> (cd /builds/worker/workspace/build/src/js/src/tests && tar -chf - jsreftest.html shell.js browser.js js-test-driver-end.js user.js non262/ shell/ test/ test262/ whatwg/ ) | (cd ../../../dist/test-stage/jsreftest/tests && tar -xf -)
[task 2019-09-09T19:58:21.362Z] 19:58:21 INFO - package-tests> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python /builds/worker/workspace/build/src/js/src/tests/jstests.py --make-manifests ../../../dist/test-stage/jsreftest/tests/
[task 2019-09-09T19:58:21.362Z] 19:58:21 INFO - package-tests> make[3]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/js/src/tests'
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.download_wpt_manifest
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> Downloading wpt manifest
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> 0:00.09 INFO Creating config file /builds/worker/workspace/build/src/obj-firefox/_tests/web-platform/wptrunner.local.ini
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> 0:00.09 INFO Creating directory /builds/worker/workspace/build/src/obj-firefox/_tests/web-platform/meta
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> 0:00.09 INFO Creating directory /builds/worker/workspace/build/src/obj-firefox/_tests/web-platform/mozilla/meta
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> 0:02.63 INFO Downloading manifest from https://index.taskcluster.net/v1/task/gecko.v2.mozilla-central.revision.de62729b366c1e8ca4a4718f433abd190b6c50f4.source.manifest-upload/artifacts/public/manifests.tar.gz
[task 2019-09-09T19:58:38.107Z] 19:58:38 INFO - package-tests> make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:38.509Z] 19:58:38 INFO - package-tests> make[2]: Entering directory '/builds/worker/workspace/build/src/obj-firefox'
[task 2019-09-09T19:58:38.510Z] 19:58:38 INFO - package-tests> /builds/worker/workspace/build/src/obj-firefox/_virtualenvs/init/bin/python -m mozbuild.action.test_archive awsy '/builds/worker/artifacts/target.awsy.tests.tar.gz'
[task 2019-09-09T19:58:38.510Z] 19:58:38 INFO - package-tests> Wrote 21 files in 21141 bytes to target.awsy.tests.tar.gz in 0.27s
...

Flags: needinfo?(gijskruitbosch+bugs)

The jar log normally contains entries like:

omni.ja path/to/file
browser/omni.ja path/to/anotherfile

So the gist here is:

[task 2019-09-09T19:58:12.494Z] 19:58:12 INFO - package> Exception: No jar log data for browser/omni.ja

This seems to be caused by all the jar logging only logging browser/omni.ja things, but without the browser path included.

Needless to say, this is very wrong. It indicates that while GreD fetched the right directory up till now, with the patch, we're fetching the wrong one. AFAICT, GreD is supposed to be the dir with firefox in it, but is actually the browser subdir with the attached patch applied.

This is doubly concerning because it might mean we're passing the wrong appdir to subprocesses after my changes in bug 1163079.
This code is all super confusing. AFAICT the code in https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#357-359 always passes the same thing for both GreD and GreBinD (which is strange because on mac, the two are supposed to be different). However, the code in https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/toolkit/xre/nsXREDirProvider.cpp#139 seems to be what overrides this. It's not clear to me why we have both of these sources of truth, or (off-hand), what the best way of unifying them is. Nathan, do you have ideas for the latter? Is this just an abstraction from when "toolkit" was an alternative to xpfe and now it can all go away? Or is there something else going on?

Flags: needinfo?(nfroyd)

(In reply to :Gijs (he/him) from comment #4)

Needless to say, this is very wrong. It indicates that while GreD fetched the right directory up till now, with the patch, we're fetching the wrong one. AFAICT, GreD is supposed to be the dir with firefox in it, but is actually the browser subdir with the attached patch applied.

This is doubly concerning because it might mean we're passing the wrong appdir to subprocesses after my changes in bug 1163079.

So I think the change in 1163079 was correct, as it replaced use of XCurrentProcD which is supposed to be the browser subdir (which doesn't make a lot of sense as that does not, in fact, contain the firefox binary... but never mind that for now).

The trick is how we're going to get the gre dir here...

This code is all super confusing. AFAICT the code in https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#357-359 always passes the same thing for both GreD and GreBinD (which is strange because on mac, the two are supposed to be different). However, the code in https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/toolkit/xre/nsXREDirProvider.cpp#139 seems to be what overrides this.

From blame, my suspicion is that the main directory service code for gred/grebind is supposed to just be dead. DougT added this fallback for when there is no GRE. We never do not have it these days, so there's no need for it. Of course, that doesn't actually solve this particular issue...

Flags: needinfo?(gijskruitbosch+bugs)

It is not super-clear how this all works, and what paths are getting taken where. My suspicion is that we are no longer going through:

https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#196-221

or this code:

https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#57-66

and are therefore not honoring providers that have been set up (though I think those are only used by test code?) or we are not getting mCurProcD, which might have been set up specially when we initialized XPCOM.

Flags: needinfo?(nfroyd)
Priority: -- → P2
Whiteboard: [necko-triaged]

Having just commented on bug 1163021 comment 22, I remembered that one other option I considered was to have the jar log code dispatch a runnable to the main thread for each jar access that we (may) want to log... but that's quite a few of those runnables and seems a bit of a waste... am I missing an obvious simpler solution?

Also, dumb question, if different threads can hit this code, and they all write to this one file from their threads... that's not threadsafe either, right? (ie, the ordering file could be a mess...).

Flags: needinfo?(nfroyd)

(In reply to Nathan Froyd [:froydnj] from comment #6)

It is not super-clear how this all works, and what paths are getting taken where. My suspicion is that we are no longer going through:

https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#196-221

or this code:

https://searchfox.org/mozilla-central/rev/8ecd15049c11eb753717a6631213ba5ecc90e123/xpcom/io/nsDirectoryService.cpp#57-66

and are therefore not honoring providers that have been set up (though I think those are only used by test code?) or we are not getting mCurProcD, which might have been set up specially when we initialized XPCOM.

Yes, with the patch as attached we're ignoring the xre dirprovider, and the builtin dirsvc code gives a different answer to the xre dirprovider, which explains the broken results.

(In reply to :Gijs (he/him) from comment #7)

Having just commented on bug 1163021 comment 22, I remembered that one other option I considered was to have the jar log code dispatch a runnable to the main thread for each jar access that we (may) want to log... but that's quite a few of those runnables and seems a bit of a waste... am I missing an obvious simpler solution?

The other alternative is to buffer all the data under a mutex, and only write things out when the zip archive is closed.

I'm not too exercised about codepaths that only get exercised under PGO generate runs, as we're not "shipping" this code and as long as they're not blatantly inefficient.

Also, dumb question, if different threads can hit this code, and they all write to this one file from their threads... that's not threadsafe either, right? (ie, the ordering file could be a mess...).

Um, yes, that seems bad. Are we really touching jar things from more than two threads (main and something else, though that's already bad...)?

But honestly, if we are touching the logger from multiple threads, it seems like we would be touching all of nsZipArchive from multiple threads, which is even more terrible...so maybe we should stop doing that?

Flags: needinfo?(nfroyd)

(In reply to Nathan Froyd [:froydnj] from comment #9)

(In reply to :Gijs (he/him) from comment #7)

Having just commented on bug 1163021 comment 22, I remembered that one other option I considered was to have the jar log code dispatch a runnable to the main thread for each jar access that we (may) want to log... but that's quite a few of those runnables and seems a bit of a waste... am I missing an obvious simpler solution?

The other alternative is to buffer all the data under a mutex, and only write things out when the zip archive is closed.

Ah, right.

I'm not too exercised about codepaths that only get exercised

I see what you did there! :-)

under PGO generate runs, as we're not "shipping" this code and as long as they're not blatantly inefficient.

Yeah, fair.

Also, dumb question, if different threads can hit this code, and they all write to this one file from their threads... that's not threadsafe either, right? (ie, the ordering file could be a mess...).

Um, yes, that seems bad. Are we really touching jar things from more than two threads (main and something else, though that's already bad...)?

I gotta admit, I didn't do any runtime logging/checks, but I'm going on the following code inspection / observations:

  • we're definitely touching this from some thread other than the mainthread (the assertion stack in comment #0 shows this) - and we do this quite early / regularly, as even the initial "start firefox to prep a profile and then exit" run of the PGO generate run fails with this assertion. At a guess, I suspect this has happened since at least bug 1373708.
  • I can see a direct callpath to OpenArchive from nsIZipReader's Open method, which is IDL exposed so all JS accesses are guaranteed to be main-thread because it's JS-based XPCOM, as well as having several C++ callers that look like they should be mainthread (dom/xhr/XMLHttpRequestMainThread.cpp sure sounds like that, anyway, and I'm pretty sure the main omni.ja init things in xpcom/build/Omnijar.cpp are, too) - we're just hoping none of them get hit at the same time, AFAICT... Basically, anything that touches zip files and doesn't directly run through nsJARChannels or the URI preloader
  • On that last note, I'm fairly sure the URL preloader (cf here) and jar channels (cf. here, which I think is what the stack in comment #0 hits) each use their own background thread, ie those are probably different, too.

So from OpenArchive, we're calling Init on the zipLog, which guards on fd, so if we're lucky then that gets called from some thread and finishes and writes to fd before any other thread calls into it (ie hopefully the first read from an archive is somewhere that blocks enough stuff that it is guaranteed to have finished before any further reads). In terms of the actual writes, I think they're all from here: https://searchfox.org/mozilla-central/rev/053579099d936e26393ac10b809b14fb5841c0f0/modules/libjar/nsZipArchive.cpp#477 . The URLPreloader definitely calls GetItem on non-mainthreads, as does the jar channel read mentioned above. XMLHttpRequestMainThread looks like it calls it on the main thread, and omni.ja exposes the zipreaders it opens and callsites like prefs I'm pretty sure also access those items from the main thread, through the URL preloader, that is, if the URL preloader doesn't have the data preloaded at that point, it'll go read it from the "normal" location, AIUI.

But honestly, if we are touching the logger from multiple threads, it seems like we would be touching all of nsZipArchive from multiple threads, which is even more terrible...so maybe we should stop doing that?

Well, I hope/assume that an individual nsZipArchive instance is never shared across more than 1 thread, but the jar logger (static ZipArchiveLogger zipLog) is a static, so it is / can be accessed from multiple zip archives if I'm not mistaken?

Anyway, assuming I'm right about this (and please check my reasoning!)... what to do. Looking at this some more, while the init and determination of whether we want to jar-log the file is made when calling OpenArchive, the actual writing is done for each GetItem call. So AIUI we'd want to dispatch to mainthread from GetItem, and hand the runnable thing we're dispatching a copy of the archive's nsIFile and the path we're reading. The jar log could cache the gredir the first time it's called and that might make things OK (assuming that doesn't sound too "blatantly inefficient" :-) ). Does that sound right? If so I can probably do a patch for that...

If we're worried about the zip archive instances being reused across threads, I'm happy to file a bug on that but I'm so many yaks away from home that I'd prefer if someone else did the actual shaving (with apologies for stretching that metaphor).

Flags: needinfo?(nfroyd)

(In reply to :Gijs (he/him) from comment #10)

(In reply to Nathan Froyd [:froydnj] from comment #9)

I'm not too exercised about codepaths that only get exercised

I see what you did there! :-)

What? Where? :)

Anyway, assuming I'm right about this (and please check my reasoning!)... what to do. Looking at this some more, while the init and determination of whether we want to jar-log the file is made when calling OpenArchive, the actual writing is done for each GetItem call. So AIUI we'd want to dispatch to mainthread from GetItem, and hand the runnable thing we're dispatching a copy of the archive's nsIFile and the path we're reading. The jar log could cache the gredir the first time it's called and that might make things OK (assuming that doesn't sound too "blatantly inefficient" :-) ). Does that sound right? If so I can probably do a patch for that...

I think this sounds like a reasonable course of action.

Flags: needinfo?(nfroyd)
Attachment #9091476 - Attachment is obsolete: true

My time is being taken up with other things and the debug assert I added in bug 1163021 means hopefully people aren't adding new versions of this, so this is lower priority - unassigning for now.

Assignee: gijskruitbosch+bugs → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(valentin.gosu)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: