From b398508ff2b9e8048630ee7a722d2ec8ecec7872 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Thu, 21 Mar 2024 17:12:43 +0000 Subject: [PATCH] Fix iframe execCtx race The events are as follows when an iframe is attached: 1. execCtx for about:blank for new frame is set; 2. execCtx for navigated frame with url is set; 3. original about:blank execCtx is destroyed. If we only work with the original execCtx, then it will be destroyed by the 3rd event. To prevent this we overwrite the original one with the new one in the 2nd event. What this means is that we do not end up in an infinite loop when waitForExecutionContext is called. --- common/frame.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/common/frame.go b/common/frame.go index e0d07a121..0857ea3c4 100644 --- a/common/frame.go +++ b/common/frame.go @@ -447,10 +447,31 @@ func (f *Frame) setContext(world executionWorld, execCtx frameExecutionContext) panic(err) } + // There is a race condition when it comes to attaching iframes and the + // execution context that apply to these frames. What usually occurs is: + // + // 1. An exec context for about:blank is first set; + // 2. A new set event is received for exec context for the url pointing + // to the actual destination for the iframe; + // 3. Finally the execution context for about:blank is destroyed, not + // for the second execution context. + // + // This is the order of events when iframes are in use on a site, and + // so it is safe to nil the original execution context and overwrite it + // with the second one. + // + // The exec context destroyed event will not remove the new exec context + // since the ids do not match. + // + // If we didn't overwrite the first execCtx with the new one, then + // waitForExecutionContext could end up waiting indefinitely since all + // execCtx were destroyed. if f.executionContexts[world] != nil { - f.log.Debugf("Frame:setContext", "fid:%s furl:%q ectxid:%d world:%s, world exists", + f.log.Debugf("Frame:setContext", "fid:%s furl:%q ectxid:%d world:%s, overriding existing world", f.ID(), f.URL(), execCtx.ID(), world) - return + + f.executionContexts[world] = nil + f.documentHandle = nil } f.executionContexts[world] = execCtx