rmottola / Arctic-Fox

Web Browser for Mac OS X 10.6+, Linux (PowerPC, x86, amd64, ARM, MIPS), NetBSD, OpenBSD, and Windows XP.
Other
290 stars 35 forks source link

search broken in toolbar and new tab #188

Closed rmottola closed 4 months ago

rmottola commented 5 months ago

This is a regression.

I suppose it comes from this commit: 1234522. But maybe more.

I made I quick fix for the toolbar, but newtab is still not working.

rmottola commented 5 months ago

@roytam1 any idea? help in finding the breaking commit for newtab?

roytam1 commented 5 months ago

@roytam1 any idea? help in finding the breaking commit for newtab?

no idea, I'm still find the cause of crash in NSS(actually NSPR, in _PR_DestroyThreadPrivate) when terminating arcticfox

rmottola commented 5 months ago

@roytam1 crash on ArcticFox shutdown? Got that, but now seems gone. I commited lots of security related stuff and perhaps I fixed it "by luck"

roytam1 commented 5 months ago

crash on ArcticFox shutdown? Got that, but now seems gone.

I can get some more crashes, for example:

stack trace:

>   xul.dll!mozilla::dom::CanvasRenderingContext2D::ClearRect(double aX, double aY, double aW, double aH) Line 2564 C++
    xul.dll!mozilla::dom::CanvasRenderingContext2DBinding::clearRect(JSContext * cx, JS::Handle<JSObject *> obj, mozilla::dom::CanvasRenderingContext2D * self, const JSJitMethodCallArgs & args) Line 3155 C++
    xul.dll!mozilla::dom::GenericBindingMethod(JSContext * cx, unsigned int argc, JS::Value * vp) Line 2731 C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 478  C++
    mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Line 2841 C++
    mozjs.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 428  C++
    mozjs.dll!js::ExecuteKernel(JSContext * cx, JS::Handle<JSScript *> script, JSObject & scopeChainArg, const JS::Value & newTargetValue, js::AbstractFramePtr evalInFrame, JS::Value * result) Line 687   C++
    mozjs.dll!js::Execute(JSContext * cx, JS::Handle<JSScript *> script, JSObject & scopeChainArg, JS::Value * rval) Line 717   C++
    mozjs.dll!Evaluate(JSContext * cx, JS::Handle<JSObject *> scope, JS::Handle<js::StaticScope *> staticScope, const JS::ReadOnlyCompileOptions & optionsArg, JS::SourceBufferHolder & srcBuf, JS::MutableHandle<JS::Value> rval) Line 4473    C++
    mozjs.dll!Evaluate(JSContext * cx, JS::AutoVectorRooter<JSObject *> & scopeChain, const JS::ReadOnlyCompileOptions & optionsArg, JS::SourceBufferHolder & srcBuf, JS::MutableHandle<JS::Value> rval) Line 4499  C++
    mozjs.dll!JS::Evaluate(JSContext * cx, JS::AutoVectorRooter<JSObject *> & scopeChain, const JS::ReadOnlyCompileOptions & optionsArg, JS::SourceBufferHolder & srcBuf, JS::MutableHandle<JS::Value> rval) Line 4560  C++
    xul.dll!nsJSUtils::EvaluateString(JSContext * aCx, JS::SourceBufferHolder & aSrcBuf, JS::Handle<JSObject *> aEvaluationGlobal, JS::CompileOptions & aCompileOptions, const nsJSUtils::EvaluateOptions & aEvaluateOptions, JS::MutableHandle<JS::Value> aRetValue, void * * aOffThreadToken) Line 213    C++
    xul.dll!nsJSUtils::EvaluateString(JSContext * aCx, JS::SourceBufferHolder & aSrcBuf, JS::Handle<JSObject *> aEvaluationGlobal, JS::CompileOptions & aCompileOptions, void * * aOffThreadToken) Line 276 C++
    xul.dll!nsScriptLoader::EvaluateScript(nsScriptLoadRequest * aRequest, JS::SourceBufferHolder & aSrcBuf) Line 1147  C++
    xul.dll!nsScriptLoader::ProcessRequest(nsScriptLoadRequest * aRequest) Line 966 C++
    xul.dll!nsScriptRequestProcessor::Run() Line 393    C++
    xul.dll!nsContentUtils::RemoveScriptBlocker() Line 5149 C++
    xul.dll!nsDocument::EndUpdate(unsigned int aUpdateType) Line 4891   C++
    xul.dll!nsHTMLDocument::EndUpdate(unsigned int aUpdateType) Line 2534   C++
    xul.dll!nsINode::ReplaceOrInsertBefore(bool aReplace, nsINode * aNewChild, nsINode * aRefChild, mozilla::ErrorResult & aError) Line 2465    C++
    xul.dll!mozilla::dom::NodeBinding::insertBefore(JSContext * cx, JS::Handle<JSObject *> obj, nsINode * self, const JSJitMethodCallArgs & args) Line 583  C++
    xul.dll!mozilla::dom::GenericBindingMethod(JSContext * cx, unsigned int argc, JS::Value * vp) Line 2731 C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 478  C++
    mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Line 2841 C++
    mozjs.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 428  C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 499  C++
    mozjs.dll!js::fun_apply(JSContext * cx, unsigned int argc, JS::Value * vp) Line 1377    C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 478  C++
    mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Line 2841 C++
    mozjs.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 428  C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 499  C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 530    C++
    mozjs.dll!js::jit::DoCallFallback(JSContext * cx, js::jit::BaselineFrame * frame, js::jit::ICCall_Fallback * stub_, unsigned int argc, JS::Value * vp, JS::MutableHandle<JS::Value> res) Line 6144  C++
    [External Code] 
    [Frames below may be incorrect and/or missing]  
    mozjs.dll!EnterBaseline(JSContext * cx, js::jit::EnterJitData & data) Line 152  C++
    mozjs.dll!js::jit::EnterBaselineAtBranch(JSContext * cx, js::InterpreterFrame * fp, unsigned char * pc) Line 256    C++
    mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Line 1854 C++
    mozjs.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 428  C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Line 499  C++
    mozjs.dll!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 530    C++
    mozjs.dll!JS::Call(JSContext * cx, JS::Handle<JS::Value> thisv, JS::Handle<JS::Value> fval, const JS::HandleValueArray & args, JS::MutableHandle<JS::Value> rval) Line 2904 C++
    xul.dll!mozilla::dom::Function::Call(JSContext * cx, JS::Handle<JS::Value> aThisVal, const nsTArray<JS::Value> & arguments, JS::MutableHandle<JS::Value> aRetVal, mozilla::ErrorResult & aRv) Line 37   C++
    xul.dll!mozilla::dom::Function::Call<nsCOMPtr<nsISupports> >(const nsCOMPtr<nsISupports> & thisVal, const nsTArray<JS::Value> & arguments, JS::MutableHandle<JS::Value> aRetVal, mozilla::ErrorResult & aRv, const char * aExecutionReason, mozilla::dom::CallbackObject::ExceptionHandling aExceptionHandling, JSCompartment * aCompartment) Line 58   C++
    xul.dll!nsGlobalWindow::RunTimeoutHandler(nsTimeout * aTimeout, nsIScriptContext * aScx) Line 11975 C++
    xul.dll!nsGlobalWindow::RunTimeout(nsTimeout * aTimeout) Line 12211 C++
    xul.dll!nsGlobalWindow::TimerCallback(nsITimer * aTimer, void * aClosure) Line 12456    C++
    xul.dll!nsTimerImpl::Fire() Line 526    C++
    xul.dll!mozilla::BackgroundHangThread::FindThread() Line 510    C++
    xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 1024 C++
    xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 297    C++
    xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate) Line 101    C++
    xul.dll!MessageLoop::RunHandler() Line 228  C++
    xul.dll!MessageLoop::Run() Line 202 C++
    xul.dll!nsBaseAppShell::Run() Line 158  C++
    xul.dll!nsAppShell::Run() Line 172  C++
    xul.dll!nsAppStartup::Run() Line 282    C++
    xul.dll!XREMain::XRE_mainRun() Line 4362    C++
    xul.dll!NS_InitXPCOM2(nsIServiceManager * * aResult, nsIFile * aBinDirectory, nsIDirectoryServiceProvider * aAppFileLocationProvider) Line 757  C++
    xul.dll!ScopedXPCOMStartup::Initialize() Line 1535  C++
    xul.dll!base::Histogram::Add(int value) Line 131    C++
    xul.dll!`anonymous namespace'::HistogramAdd(base::Histogram & histogram, int value, unsigned int dataset) Line 1187 C++
    xul.dll!XRE_TelemetryAccumulate(int aID, unsigned int aSample) Line 3911    C++
    [External Code] 
void
CanvasRenderingContext2D::ClearRect(double aX, double aY, double aW,
                                    double aH)
{
  // Do not allow zeros - it's a no-op at that point per spec.
  if (!ValidateRect(aX, aY, aW, aH, false)) {
    return;
  }

  mTarget->ClearRect(gfx::Rect(aX, aY, aW, aH)); // <-- mTarget is nullptr here

while I addressed this in my repo: https://github.com/roytam1/palemoon27/commit/13e8a4e26a3e24893d30fd8d8cdbdeb08d5d1222

and the crash when terminating is still reproducible:

stack trace:

    583ea5c2()  Unknown
    [Frames below may be incorrect and/or missing]  
>   nss3.dll!_PR_DestroyThreadPrivate(PRThread * self) Line 237 C
    nss3.dll!_PR_CleanupThread(PRThread * thread) Line 33   C
    nss3.dll!_PR_NativeRunThread(void * arg) Line 427   C
    nss3.dll!pr_root(void * arg) Line 95    C
    [External Code] 

debug log: Unhandled exception at 0x583EA5C2 in arcticfox.exe: 0xC0000005: Access violation executing location 0x583EA5C2.

void _PR_DestroyThreadPrivate(PRThread* self)
{
#define _PR_TPD_DESTRUCTOR_ITERATIONS 4

    if (NULL != self->privateData)  /* we have some */
    {
        PRBool clean;
        PRUint32 index;
        PRInt32 passes = _PR_TPD_DESTRUCTOR_ITERATIONS;
        PR_ASSERT(0 != self->tpdLength);
        do
        {
            clean = PR_TRUE;
            for (index = 0; index < self->tpdLength; ++index)
            {
                void *priv = self->privateData[index];  /* extract */
                if (NULL != priv)  /* we have data at this index */
                {
                    if (NULL != _pr_tpd_destructors[index])
                    {
                        self->privateData[index] = NULL;  /* precondition */
                        (*_pr_tpd_destructors[index])(priv);  /* destroy */ // <-- crash here
                        clean = PR_FALSE;  /* unknown side effects */
                    }
                }

possible cause: DLL is unloaded while it is still running: https://stackoverflow.com/questions/61312902/unloading-dll-throws-an-access-violation-error

but I'm not able to find which rev is guilty at the moment.

rmottola commented 4 months ago

@roytam1 I think the current version has the shutdown crash fixed. Seems stable for me, I tried several times.

However, all the new patches seem to have made AF prone to crash with certain pages, consistently the same. I was unable to get a signifcant backtrace.

roytam1 commented 4 months ago

@roytam1 I think the current version has the shutdown crash fixed. Seems stable for me, I tried several times.

still happens here, try visiting https://www.jaynestars.com/ , scrolling to bottom with mouse wheel, then close browser.

rmottola commented 4 months ago

@roytam1 tried and it crashes before reaching the bottom ! darn

roytam1 commented 4 months ago

@roytam1 tried and it crashes before reaching the bottom ! darn

you may hit CanvasRenderingContext2D::ClearRect() null mTarget crash first if you don't handle empty mTarget in ClearRect.

rmottola commented 4 months ago

@roytam1 tried and it crashes before reaching the bottom ! darn

you may hit CanvasRenderingContext2D::ClearRect() null mTarget crash first if you don't handle empty mTarget in ClearRect.

oh... right, I found the culprit. I removed accidentally an EnsureTarget() during a patch merge. https://github.com/rmottola/Arctic-Fox/commit/32fa5b83374a5272590d324d9f0340c86192b78b

fixes it, it should make your other patch unnecessary. With this I can scroll the document down (it never finishes loading as many sites) quite ArcticFox and have no crash.

rmottola commented 4 months ago

@roytam1 shutdown-crash solved now for you? It is for me, seems consistent.

So the original issue here, that one is still open for me.

roytam1 commented 4 months ago

@roytam1 shutdown-crash solved now for you?

haven't test here, will merge tomorrow and see if it still crash or not.

EDIT: tested and same happens.

rmottola commented 4 months ago

It all looks fixed on my side, visiting https://www.jaynestars.com/ does not cause a crash on closure anymore, nor on Linux nor on MacOS. Perhaps you have some more changes you better review or take out since they are not needed? I hope it didn't became Windows specific.

rmottola commented 4 months ago

@roytam1 the original bug is actually fixed too! So I would close here after your input. If it persists, it would be best to make a new issue and collect all the "crashy sites" as test cases.

roytam1 commented 4 months ago

@roytam1 the original bug is actually fixed too! So I would close here after your input. If it persists, it would be best to make a new issue and collect all the "crashy sites" as test cases.

yeah I think this bug can be closed. and NSPR crash bug when terminating may be not related to any sites, and may be only happen in windows.

crashing thread is a Cache2 I/O thread:

startFunc = 0x58729ab0 {xul.dll!mozilla::net::CacheIOThread::ThreadFunc(void *)}
name = 0x09a4d190 "Cache2 I/O"
rmottola commented 4 months ago

@roytam1 hard with stack-trace. Please sync, update, check that you may have added changes not needed anymore and open separate bug. I suppose on windows NSPR is the AF supplied one and not system. I will test both.