How to bind AssociatedInterfacePtr to /dev/null?

[luk...@chromium.org]

Hello,

Context:

I am trying to postpone when we start queuing IPCs to a renderer.  Current behavior: queueing starts when RenderProcessHostImpl is constructed.  Desired behavior (I think): queueing starts when RenderProcessHostImpl::Init is called.  The motivation here is https://crbug.com/813045

If queueing starts in RPHI::Init, then I need to decide what is the state of RPHI::GetRendererInterface (mojom::RendererPtr) prior to the RPHI::Init call.  I think this state should be consistent with the state of RPHI::GetRendererInterface after a renderer process crashes.  AFAICT, RPHI::ProcessDied doesn't reset RPHI::renderer_interface_ and so RPHI::GetRendererInterface keeps returning a non-null value but all IPCs are discarded.

 

I am currently trying to replicate this behavior (this behavior = non-null RPHI::GetRendererInterface that discards IPCs) into a freshly constructed RPHI, but I guess we can also discuss returning null from RPHI::GetRendererInterface in both cases (case1: freshly constructed RPHI;  case2: RPHI wrapping a crashed/dead/gone process).  The main reason I went with a non-null/discarding approach is that 1) quite a few cases are happy with discarding an IPC and 2) I haven't yet encountered a case where the IPC should be queued (and the code path is broken because it doesn't guarantee that RPHI::Init will be called).

Note that similar decision/discussion also applies to RenderFrameHostImpl::GetRemoteInterfaces(). 

 

Question:

How do I bind an associated interface ptr to a pipe connected to /dev/null?

This is what I have for regular interface ptrs:

template <typename T>

void BindInterfaceToClosedPipe(T* interface_ptr) {

  typename T::PtrInfoType info(mojo::ScopedMessagePipeHandle(), 0u);

  interface_ptr->Bind(std::move(info));

} 

This is what I tried for associated interface ptrs:

template <typename T>

void BindAssociatedInterfaceToClosedPipe(T* associated_interface_ptr) {

  mojo::ScopedInterfaceEndpointHandle h1, h2;

  mojo::ScopedInterfaceEndpointHandle::CreatePairPendingAssociation(&h1, &h2);

  typename T::PtrInfoType info(std::move(h2), 0u);

  associated_interface_ptr->Bind(std::move(info));

}

This fails with:

 [196374:196374:0604/112350.225723:FATAL:interface_endpoint_client.cc(227)] Check failed: !handle_.pending_association(). 

#0 0x7f7ef461f61c base::debug::StackTrace::StackTrace()

#1 0x7f7ef45696db logging::LogMessage::~LogMessage()

#2 0x7f7ef4f75f38 mojo::InterfaceEndpointClient::Accept()

#3 0x7f7ef1f597ed content::mojom::RendererProxy::SetIsLockedToSite()

... 

#5 0x7f7ef278e5b5 content::SiteInstanceImpl::LockToOriginIfNeeded() 

... 

 

Thanks,

Lukasz

 

 

 

 

 

[Lucas Gadani]

Maybe MakeRequestAssociatedWithDedicatedPipe (https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/associated_interface_ptr.h?l=230)  would work for you?

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/e073873c-2c54-494f-b563-02e5e76b4e0e%40chromium.org.

[Ken Rockot]

What Lucas said is really the only option, though I find it pretty disagreeable that we'd ever use that API in non-testing code. Someone recently tripped over this unnecessary use of it, for example, and I imagine a lot of time was wasted trying to figure out what was going on.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALQVofpeN%2B%3DD2f4gYOmgaMJ5h9-FE040P51JqgbGNKAd4MSKYQ%40mail.gmail.com.

[Łukasz Anforowicz]

On Mon, Jun 4, 2018 at 12:03 PM, Ken Rockot <roc...@chromium.org> wrote:

What Lucas said is really the only option, though I find it pretty disagreeable that we'd ever use that API in non-testing code. Someone recently tripped over this unnecessary use of it, for example, and I imagine a lot of time was wasted trying to figure out what was going on.

So, you'd suggest to instead tweak RenderProcessHostImpl::ProcessDied so that it calls renderer_interface_.reset() instead?  Did I get that right?

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/e073873c-2c54-494f-b563-02e5e76b4e0e%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

[Ken Rockot]

I think so yes. Probably a bit trickier since that means GetRendererInterface() can no longer be called indiscriminately, but at least callers will have to explicitly acknowledge that there might not be a process to communicate with.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/e073873c-2c54-494f-b563-02e5e76b4e0e%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALQVofpeN%2B%3DD2f4gYOmgaMJ5h9-FE040P51JqgbGNKAd4MSKYQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.

To post to this group, send email to chromi...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAA_NCUGUqjQkAMopay5a4TExVHef_Z2X7bOARj%3DvX16GV6OKOA%40mail.gmail.com.

[Daniel Cheng]

IMO, there is a pretty big downside to making things optional: in Blink, we have many things that return values by pointer. Over time, it's become quite unclear when a pointer can be null and when it might not be. Often, it's tempting to add pre-emptive "just in case this is null" checks: this fixes the immediate problem of preventing crashes, but makes it quite hard to understand the code in the long-term.

Many callers simply don't care about the distinction (and the fact that we reuse the RenderProcessHost across crashes is also an attempt to abstract this detail away); why not just make the callsites that do care do an explicit call to RPH::IsReady()?

Daniel

[Łukasz Anforowicz]

If we do |renderer_interface_.reset() | in RenderProcessHostImpl::ProcessDied, then it means that interfaces that used to be bound to the dead process are treated inconsistently: some of the interfaces have been reset (and require a null check before using them), while other interfaces are just discarding their messages (e.g. whatever random interfaces was obtained via RFHI::GetRemoteInterfaces). [unless those random interfaces happen to have an error handler that does the resetting]

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/e073873c-2c54-494f-b563-02e5e76b4e0e%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.

To post to this group, send email to chromi...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALQVofpeN%2B%3DD2f4gYOmgaMJ5h9-FE040P51JqgbGNKAd4MSKYQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.