826931 - B2G RIL: use UnixSocket in SystemWorkerManager

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

As suggested in bug 814579 comment #18, we should spend some time study whether it's possible to connect rilproxy using common ipc utility component UnixSocket, which was invented for Bluetooth.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Depends on bug 814581 so that it's easier to find out what's the final requirement for RIL socket connections.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Just make it work first in single sim setup so that I can skip ref counting stuff in bug 814579.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData()

RIL have to process the received UnixSocketRawData object in worker thread, but the original SocketReceiveTask keeps the ownership and destroys that object almost right after ReceiveSocketData() is called.

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 2/3: UnixSocket - allow delayed connection

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3/3: use mozilla::ipc::UnixSocket

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

Depends on bug 826977 because above patches are based on the previous works.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 704658 [details] [diff] [review]
Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData()

Review of attachment 704658 [details] [diff] [review]:
-----------------------------------------------------------------

If you're going to share ownership, just switch to refcount. Passing around autoptr refs is confusing.

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 704659 [details] [diff] [review]
Part 2/3: UnixSocket - allow delayed connection

Review of attachment 704659 [details] [diff] [review]:
-----------------------------------------------------------------

Just curious, why do we need to delay initial connection?

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 704660 [details] [diff] [review]
Part 3/3: use mozilla::ipc::UnixSocket

Review of attachment 704660 [details] [diff] [review]:
-----------------------------------------------------------------

General idea looks ok so fb+'ing, but will need a revision for the refcount stuff.

::: ipc/ril/Ril.cpp
@@ +41,3 @@
>  
>  private:
> +    nsAutoPtr<UnixSocketRawData> mMessage;

See comment on patch 1 about making this refcounted. autoptr is just bad news.

@@ +60,3 @@
>  }
>  
> +class RilConnector : public mozilla::ipc::UnixSocketConnector

I think you could go one step farther abstract with this and just call it "UnixDomainSocketConnector", and put it over in ipc/unixsocket. I don't believe it's doing anything RIL specific outside of the debug port stuff, and honestly that might need to be yanked since we could have multiple RIL processes colliding while all trying to bind to the same port if someone tries to do debugging on multiple processes. 

However, works here for now. We can always make it more abstract in a followup.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #8)
> Comment on attachment 704659 [details] [diff] [review]
> Part 2/3: UnixSocket - allow delayed connection
> -----------------------------------------------------------------
> Just curious, why do we need to delay initial connection?

I didn't delay the initial connection but reconnections.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2

Use nsRefPtr instead.

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 2/3: UnixSocket - allow delayed connection : v2

Re-gen patch from hg.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3/3: use mozilla::ipc::UnixSocket : v2

1) Use nsRefPtr instead.
2) Connect to different port in desktop debug.

Comment 14

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 705289 [details] [diff] [review]
Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2

Eric, I finally get something needs your review!

Comment 15

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #13)
> 2) Connect to different port in desktop debug.

Sorry, that's in bug 814579.

Comment 16

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 705289 [details] [diff] [review]
Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2

Review of attachment 705289 [details] [diff] [review]:
-----------------------------------------------------------------

Almost there.

::: ipc/unixsocket/UnixSocket.h
@@ +129,5 @@
>     * main thread.
>     *
>     * @param aMessage Data received from the socket.
>     */
> +  virtual void ReceiveSocketData(nsRefPtr<UnixSocketRawData> aMessage) = 0;

And now this (and all other parameters) can change back to a regular old UnixSocketRawData* since we pass refcounted pointers as bare usually.

Comment 17

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 705291 [details] [diff] [review]
Part 2/3: UnixSocket - allow delayed connection : v2

Review of attachment 705291 [details] [diff] [review]:
-----------------------------------------------------------------

I'm a little iffy on putting that delay down in UnixSocket when it's in the context of RIL's reconnects. Why not make the delay for the connection task in RIL itself, instead of moving it to UnixSocket?

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #17)
> Comment on attachment 705291 [details] [diff] [review]
> Part 2/3: UnixSocket - allow delayed connection : v2
-----------------------------------------------------------------
> I'm a little iffy on putting that delay down in UnixSocket
> when it's in the context of RIL's reconnects. Why not make the
> delay for the connection task in RIL itself, instead of moving
> it to UnixSocket?

But, why not? You can find both Netd & coming WebNfc socket connection implementations are just copied from RIL's. I rewrite RIL parts in this patch, will/should they follow as well in the future? Then they have exactly the same problem, delay at retry. It's not something RIL-only, and it's also a nice feature to have in such task-driven model.

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #16)
> Comment on attachment 705289 [details] [diff] [review]
> Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2
-----------------------------------------------------------------
> And now this (and all other parameters) can change back to a
> regular old UnixSocketRawData* since we pass refcounted pointers
> as bare usually.

What? what do you mean exactly?

Comment 20

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #18)
> (In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #17)
> > Comment on attachment 705291 [details] [diff] [review]
> > Part 2/3: UnixSocket - allow delayed connection : v2
> -----------------------------------------------------------------
> > I'm a little iffy on putting that delay down in UnixSocket
> > when it's in the context of RIL's reconnects. Why not make the
> > delay for the connection task in RIL itself, instead of moving
> > it to UnixSocket?
> 
> But, why not? You can find both Netd & coming WebNfc socket connection
> implementations are just copied from RIL's. I rewrite RIL parts in this
> patch, will/should they follow as well in the future? Then they have exactly
> the same problem, delay at retry. It's not something RIL-only, and it's also
> a nice feature to have in such task-driven model.

Ah, ok, if it's getting repeated places then I guess we might as well throw it down here. I'll reverse my r-.

Comment 21

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 705291 [details] [diff] [review]
Part 2/3: UnixSocket - allow delayed connection : v2

Review of attachment 705291 [details] [diff] [review]:
-----------------------------------------------------------------

Reversing r- now that I have more context for this.

Comment 22

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #19)
> (In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #16)
> > Comment on attachment 705289 [details] [diff] [review]
> > Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2
> -----------------------------------------------------------------
> > And now this (and all other parameters) can change back to a
> > regular old UnixSocketRawData* since we pass refcounted pointers
> > as bare usually.
> 
> What? what do you mean exactly?

Notice other places we don't have nsRefPtr<T>/nsCOMPtr<T> as an in parameter, we usually have T*. However, since I can't seem to form complete sentences about this today due to having a cold, I'm going to cut and paste dholbert's explanation from IRC. :)

"basically, because the nsRefPtr wrapper is unnecessary in an argument. If the caller has a nsRefPtr, then it's presumably holding a reference for the duration of the function it's calling. if you made the parameter a nsRefPtr regardless of that, then there'd be an unnecessary addref on instantiation, release on function-return."

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #22)
> if you made the parameter a nsRefPtr regardless of that, then
> there'd be an unnecessary addref on instantiation, release on
> function-return."

We use smart pointers because they help us to manage memory without intentional works. I can surely remove the nsRefPtr wrapper, as I just added in the comments, the ownership of the received data now belongs to the callee function. It follows: I must add a lot "delete aMessage;" before all the return statements in all ReceiveSocketData implementations. A simple one-liner vs. memory leak hunting. Are you sure the latter is what you really want?

Comment 24

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 705289 [details] [diff] [review]
Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v2

Review of attachment 705289 [details] [diff] [review]:
-----------------------------------------------------------------

Almost there.

::: ipc/unixsocket/UnixSocket.h
@@ +129,5 @@
>     * main thread.
>     *
>     * @param aMessage Data received from the socket.
>     */
> +  virtual void ReceiveSocketData(nsRefPtr<UnixSocketRawData> aMessage) = 0;

And now this (and all other parameters) can change back to a regular old UnixSocketRawData* since we pass refcounted pointers as bare usually.

Comment 25

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Bringing in cjones for feedback on this because now I'm just confused as to how we want to deal with passing data like this across threads with the different ns*Ptr types.

Comment 26

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Vicamo: I talked to Ben about this yesterday, and you're right, my idea of using T* was only for passing things that don't require ownership change. That said, we can't pass nsRefPtr<T> either, because (assuming I'm remembering the conversation correctly) that's going to create a temporary that we'll take ownership of. I think we need nsRefPtr<T>&, at which point we can use .forget() inside the function? At which point this isn't a hell of a lot better than the original nsAutoPtr solution I realize, I just worry about nsAutoPtr because I've run into a lot of nasty "Oh it deleted /there/?" crashes when passing them like that before.

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #26)
> That said, we can't pass nsRefPtr<T> either, because (assuming
> I'm remembering the conversation correctly) that's going to
> create a temporary that we'll take ownership of.

The root problem passing pointers between threads is double-free. Anything may result in double-free must be avoided.

We can pass raw pointers, and the caller must not free them as some kind of convention like what we have in SendSocketData(). That's an implicit convention, and an implicit convention cannot prevent double-free in a active way.

We can nsRefPtr, but actually they're even worse than raw pointers because now they *always* try to free carried data in their destructors. Passing nsRefPtr here is for easier memory management in one thread, not for cross thread ownership transition. By passing the nsRefPtr into ReceiveSocketData(), we'll then have a similar implicit convention: the caller must forget() first. We can easily achieve this because there is only one caller has ever invoked this call -- SocketReceiveTask. But we have three callees in dom/bluetooth and one in ipc/ril. Passing nsRefPtr may make their life easier.

Passing a reference to nsRefPtr instead doesn't make the problem any easier because there must still be a nsRefPtr instance somewhere so that you can take its reference. The only difference is references may save you a few cpu cycles. That's it.

> I think we need nsRefPtr<T>&, at which point we can use
> .forget() inside the function?

I'll still suggest .forget() before calling ReceiveSocketData(), which means pass-by-value not pass-by-reference, because I don't really know when and in which thread does a SocketReceiveTask get destructed. Just want to make sure the ownership is always transferred to the callee functions.

> At which point this isn't a hell of a lot better than the
> original nsAutoPtr solution I realize, I just worry about
> nsAutoPtr because I've run into a lot of nasty "Oh it deleted
> /there/?" crashes when passing them like that before.

A `.forget()` call can be much much more clear than passing a reference.

Comment 28

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Can someone summarize what the goal is here?  "Ownership change" and "cross thread" is a large universe of things, many of them very scary.  Talking about nsRefPtr mechanics before describing the ownership model is putting the cart before the horse.

Comment 29

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #28)
> Can someone summarize what the goal is here?

We have a memory buffer allocated in a base class of thread A and passed to one of the derived classes of thread B, and it may again be passed to thread C.

1) The original method uses nsAutoPtr to hold the buffer in base class.
2) The method in my part 1 uses nsRefPtr instead and calls forget() before passing to thread B or C.

Comment 30

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

 - by "passed to thread B", do you mean that the sole ownership is transferred to thread B, or that thread B is given a reference to the buffer?  Same with the B -> C interaction.

 - how are you passing the buffers between threads?

Comment 31

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #30)
>  - by "passed to thread B", do you mean that the sole ownership is
> transferred to thread B, or that thread B is given a reference to the
> buffer?  Same with the B -> C interaction.

The original code passed raw pointer to thread B and stored it in a nsAutoPtr. The pointer was only freed in thread B.

  (thread A) UnixSocketRawData* p = new UnixSocketRawData();
             task = new SocketReceiveTask(p); // Stores in mRawData, which is a nsAutoPtr.
  (thread B) this->mImpl->mConsumer->ReceiveSocketData(mRawData); // Cast to raw pointer.
             // Do something on the raw pointer but never free it.
             // Then destructor of SocketReceiveTask frees mRawData.

When rewriting RIL code with UnixSocket, I use nsRefPtr throughout the process:

  (thread A) nsRefPtr<UnixSocketRawData> p = new UnixSocketRawData();
             task = new SocketReceiveTask(p.forget()); // Stores in mRawData.
  (thread B) this->mImpl->mConsumer->ReceiveSocketData(mRawData.forget());
             ...
             nsRefPtr<DispatchRILEvent> dre(new DispatchRILEvent(aMessage.forget()));
  (thread C) // Do somthing with DispatchRILEvent::mMessage.

>  - how are you passing the buffers between threads?

As shown above.

Comment 32

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This sharing scheme is quite simple, single owner, and using refcounting will require atomic addref/release, so I'm tempted to say that the old way was better.  That there's only one owner at any time is explicit, and there's no need for atomic inc/dec of a refcount.

(You could avoid atomic inc/dec in the second scheme but it would very delicate, and getting it wrong would lead to sec:crit bugs.)

Comment 33

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #32)
> This sharing scheme is quite simple, single owner, and using refcounting
> will require atomic addref/release, so I'm tempted to say that the old way
> was better.  That there's only one owner at any time is explicit, and
> there's no need for atomic inc/dec of a refcount.

Kyle, then should I revert first part to previous attachment 704658 [details] [diff] [review]?

Comment 34

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Yup, go for it, and I will now make a rule that I am no longer allowed to take on memory ownership reviews while ill. :)

Comment 35

[Vicamo Yang [:vicamo][:vyang]]

Vincent, after some dirty works[1] on including netd code rewrites, I don't have a simple solution to include LineWatcher support in UnixSocket yet :( So I'm renaming this bug to B2G RIL specific.

[1]: https://github.com/vicamo/b2g_releases-mozilla-central/commit/0d11350

Comment 36

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 1/3: UnixSocket - allow ownership take-over in ReceiveSocketData() : v3

Reverse to previous nsAutoPtr version (attachment #704658 [details] [diff] [review]). Rebase & re-gen patch from hg only.

Comment 37

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3/3: use mozilla::ipc::UnixSocket : v3

1) Revert to previous nsAutoPtr version (attachment 704660 [details] [diff] [review]). Re-gen patch by hg.
2) Fix one possible memory leak in SystemWorkerManager::SendRilRawData().

Comment 38

[Benoit Jacob [:bjacob] (mostly away)]

With these patches, I get a main-thread assertion failure on startup:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 493.504]
0x421c6c6a in mozilla::ipc::UnixSocketConsumer::SendSocketData (this=0x45c9c4a0, aData=0x499caea0)
    at /hack/mozilla-graphics/ipc/unixsocket/UnixSocket.cpp:586
586       MOZ_ASSERT(NS_IsMainThread());
(gdb) bt
#0  0x421c6c6a in mozilla::ipc::UnixSocketConsumer::SendSocketData (this=0x45c9c4a0, aData=0x499caea0)
    at /hack/mozilla-graphics/ipc/unixsocket/UnixSocket.cpp:586
#1  0x417ff65e in mozilla::dom::gonk::SystemWorkerManager::SendRilRawData (aRaw=0x499caea0)
    at /hack/mozilla-graphics/dom/system/gonk/SystemWorkerManager.cpp:417
#2  0x417fe72e in PostToRIL (cx=0x40442400, argc=1, vp=0x45e001a0)
    at /hack/mozilla-graphics/dom/system/gonk/SystemWorkerManager.cpp:116
#3  0x4291fdb2 in js::CallJSNative (cx=0x40442400, native=0x417fe575 <PostToRIL>, args=...)
    at /hack/mozilla-graphics/js/src/jscntxtinlines.h:353
#4  0x42927ea2 in js::InvokeKernel (cx=0x40442400, args=..., construct=js::NO_CONSTRUCT)
    at /hack/mozilla-graphics/js/src/jsinterp.cpp:390
#5  0x4292f50e in js::Interpret (cx=0x40442400, entryFrame=0x45e00110, interpMode=js::JSINTERP_NORMAL)
    at /hack/mozilla-graphics/js/src/jsinterp.cpp:2376

Comment 39

[Benoit Jacob [:bjacob] (mostly away)]

Looks like this main-thread-only C++ code is called from a Web Worker?

Comment 40

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3/3: use mozilla::ipc::UnixSocket : v4

Fix sending ril data from worker thread. Should pass to main thread first.

Comment 41

[Vicamo Yang [:vicamo][:vyang]]

try: https://tbpl.mozilla.org/?tree=Try&rev=1fb9aa7ef5e5

Comment 42

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Part 3/3: use mozilla::ipc::UnixSocket : v5

Fix build failure in debug build. Verified on Unagi. Full try again: https://tbpl.mozilla.org/?tree=Try&rev=4ae543afb135

Comment 43

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #41)
> try: https://tbpl.mozilla.org/?tree=Try&rev=1fb9aa7ef5e5

Cancelled.

Comment 44

[Mozilla RelEng Bot]

Try run for 1fb9aa7ef5e5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=1fb9aa7ef5e5
Results (out of 246 total builds):
    exception: 54
    success: 125
    warnings: 10
    failure: 57
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1fb9aa7ef5e5

Comment 45

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17e87a6b7c3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/f702f4adea06
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cf5d7935060

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/17e87a6b7c3f
https://hg.mozilla.org/mozilla-central/rev/f702f4adea06
https://hg.mozilla.org/mozilla-central/rev/8cf5d7935060

Comment 47

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I got this compile error when trying to build B2G Desktop:

/mozilla-central/ipc/ril/Ril.cpp: In member function ‘virtual int {anonymous}::RilConnector::Create()’:
/mozilla-central/ipc/ril/Ril.cpp:91:35: error: ‘gethostbyname’ was not declared in this scope
/mozilla-central/ipc/ril/Ril.cpp:93:23: error: invalid use of incomplete type ‘struct {anonymous}::RilConnector::Create()::hostent’
/mozilla-central/ipc/ril/Ril.cpp:89:12: error: forward declaration of ‘struct {anonymous}::RilConnector::Create()::hostent’
/mozilla-central/ipc/ril/Ril.cpp: In member function ‘virtual void {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)’:
/mozilla-central/ipc/ril/Ril.cpp:128:24: error: aggregate ‘{anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::sockaddr_in addr_in’ has incomplete type and cannot be defined
/mozilla-central/ipc/ril/Ril.cpp:130:35: error: ‘gethostbyname’ was not declared in this scope
/mozilla-central/ipc/ril/Ril.cpp:136:28: error: invalid use of incomplete type ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’
/mozilla-central/ipc/ril/Ril.cpp:127:12: error: forward declaration of ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’
/mozilla-central/ipc/ril/Ril.cpp:137:43: error: ‘htons’ was not declared in this scope
/mozilla-central/ipc/ril/Ril.cpp:138:33: error: invalid use of incomplete type ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’
/mozilla-central/ipc/ril/Ril.cpp:127:12: error: forward declaration of ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’
/mozilla-central/ipc/ril/Ril.cpp:138:45: error: invalid use of incomplete type ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’
/mozilla-central/ipc/ril/Ril.cpp:127:12: error: forward declaration of ‘struct {anonymous}::RilConnector::CreateAddr(bool, socklen_t&, sockaddr*, const char*)::hostent’

Comment 48

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

another 
../../dom/system/gonk/SystemWorkerManager.o: In function `mozilla::dom::gonk::SystemWorkerManager::SendRilRawData(mozilla::ipc::UnixSocketRawData*)':
/home/allstars/src/mozilla/mozilla-central/dom/system/gonk/SystemWorkerManager.cpp:436: undefined reference to `mozilla::ipc::UnixSocketConsumer::SendSocketData(mozilla::ipc::UnixSocketRawData*)'
../../ipc/ril/Ril.o: In function `mozilla::ipc::RilConsumer::OnConnectError()':
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.cpp:200: undefined reference to `mozilla::ipc::UnixSocketConsumer::CloseSocket()'
../../ipc/ril/Ril.o: In function `mozilla::ipc::RilConsumer::OnDisconnect()':
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.cpp:208: undefined reference to `mozilla::ipc::UnixSocketConsumer::ConnectSocket(mozilla::ipc::UnixSocketConnector*, char const*, int)'
../../ipc/ril/Ril.o: In function `mozilla::ipc::RilConsumer::~RilConsumer()':
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.h:20: undefined reference to `mozilla::ipc::UnixSocketConsumer::~UnixSocketConsumer()'
../../ipc/ril/Ril.o: In function `mozilla::ipc::RilConsumer::RilConsumer(mozilla::dom::workers::WorkerCrossThreadDispatcher*)':
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.cpp:168: undefined reference to `mozilla::ipc::UnixSocketConsumer::UnixSocketConsumer()'
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.cpp:170: undefined reference to `mozilla::ipc::UnixSocketConsumer::ConnectSocket(mozilla::ipc::UnixSocketConnector*, char const*, int)'
../../ipc/ril/Ril.o: In function `mozilla::ipc::RilConsumer::Shutdown()':
/home/allstars/src/mozilla/mozilla-central/ipc/ril/Ril.cpp:177: undefined reference to `mozilla::ipc::UnixSocketConsumer::CloseSocket()'

Comment 49

[Vicamo Yang [:vicamo][:vyang]]

Attachment: (follow-up): include netdb.h for |gethostbyname|

Comment 50

[Vicamo Yang [:vicamo][:vyang]]

try build on all platforms: https://tbpl.mozilla.org/?tree=Try&rev=01586c7ac5e1

Comment 51

[Mozilla RelEng Bot]

Try run for 01586c7ac5e1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=01586c7ac5e1
Results (out of 18 total builds):
    success: 17
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-01586c7ac5e1

Comment 52

[Vicamo Yang [:vicamo][:vyang]]

Attachment: (follow-up): fix break build in browser app with --enable-b2g-ril

try https://tbpl.mozilla.org/?tree=Try&rev=6bb8af75ba3d

Comment 53

[Vicamo Yang [:vicamo][:vyang]]

Attachment: (follow-up): fix break build in browser app with --enable-b2g-ril

try https://tbpl.mozilla.org/?tree=Try&rev=7cb4dc356c65

Comment 54

[Vicamo Yang [:vicamo][:vyang]]

Attachment: (follow-up): fix break build in browser app with --enable-b2g-ril

r=me instead.

Comment 55

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb0db0c171d3

Comment 56

[Dietrich Ayala (:dietrich)]

Not blocking on multisim for 1.1

Comment 57

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fb0db0c171d3

Comment 58

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Requesting tef+ and uplift ONLY FOR PATCH 1 of this bug because this blocks 851046, see reasoning in https://bugzilla.mozilla.org/show_bug.cgi?id=851046#c50

Comment 59

[Daniel Coloma:dcoloma]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #58)
> Requesting tef+ and uplift ONLY FOR PATCH 1 of this bug because this blocks
> 851046, see reasoning in
> https://bugzilla.mozilla.org/show_bug.cgi?id=851046#c50

Can I mark only part of the bug as a blocker? or should we split this into two bugs?

Comment 60

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

I was wondering about that too. I can go ahead and make a new bug with just that patch, that might be easier.

Comment 61

[Daniel Coloma:dcoloma]

(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #60)
> I was wondering about that too. I can go ahead and make a new bug with just
> that patch, that might be easier.

OK, let me know when done so I can block on it.