745349 - Various run-only-one-instance bugs in media/webrtc/signaling

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

There are several instances of using msg queues based on a non-unique identifier/filename in sipcc, such as ftok("/tmp",id), IPC filenames for sockets, etc.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: remove a set of dependencies on non-unique files/queue names in sipcc

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Verified on Fedora 15 that the cprCreateMessageQueue() creates unique queues using ftok("/proc/self"...) when multiple instances are run at once.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

I don't think I need feedback from more than one of you, but I think any of you could review this patch.

Comment 4

[enda mannion (:emannion)]

My initial reaction is this needs testing straight away as any changes to msg queue or socket needs to be very delicate or may cause harm.  I would like discuss this work some more when we ext meet.

Comment 5

[enda mannion (:emannion)]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

does using getpid() for the socket name give us uniqueness when FF runs on one process?

Comment 6

[Ethan Hugg [:ehugg]]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

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

I was able to run two instances on 64bit Ubuntu 11.10 with the same behavior as Randell's Fedora15 box.  So it appears to work.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

(In reply to enda mannion (:emannion) from comment #5)
> Comment on attachment 614921 [details] [diff] [review]
> remove a set of dependencies on non-unique files/queue names in sipcc
> 
> does using getpid() for the socket name give us uniqueness when FF runs on
> one process?

getpid() will be the same for the same firefox, even if you have separate tabs/instances.  So this does not make them unique if we're to run multiple instances within the same FF.  Easiest solution there would be to either use the ftok("/proc/self",counter) trick with a global counter we increment, or combine pid_t with a counter (snprintf(foo,"%s_%d_%u",name,pid,counter)).  pid keeps the different copies running on the system from colliding (as /proc/self does with ftok()).

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

I'm going to wait on Enda's comments per our last exchange before committing to alder/default.  Suhas, feel free to chime in too.

Comment 9

[enda mannion (:emannion)]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

No major comments on this except I would be keen to see this tested on Mac and maybe Windows ASAP.   A more general comment while going through code like this there are still some unused lines that could be tidied up as we see them for example.  In cpr_win_socket.h

 /* End of nokelly's adds */
 
 
 //extern const cpr_in6_addr_t in6addr_any;

Not a big issue.

Comment 10

[enda mannion (:emannion)]

Comment on attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc

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

This has been outstanding for way too long.

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

http://hg.mozilla.org/projects/alder/rev/ae0ba897db5c