Last Comment Bug 745349 - Various run-only-one-instance bugs in media/webrtc/signaling
: Various run-only-one-instance bugs in media/webrtc/signaling
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: WebRTC: Signaling (show other bugs)
: Trunk
: All All
: -- major (vote)
: ---
Assigned To: Randell Jesup [:jesup]
: Jason Smith [:jsmith]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-13 14:58 PDT by Randell Jesup [:jesup]
Modified: 2012-10-04 02:54 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
remove a set of dependencies on non-unique files/queue names in sipcc (12.76 KB, patch)
2012-04-13 14:59 PDT, Randell Jesup [:jesup]
ethanhugg: feedback+
emannion: feedback+
Details | Diff | Splinter Review

Description Randell Jesup [:jesup] 2012-04-13 14:58:20 PDT
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] 2012-04-13 14:59:37 PDT
Created attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc
Comment 2 Randell Jesup [:jesup] 2012-04-15 20:10:16 PDT
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] 2012-04-15 20:11:56 PDT
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) 2012-04-16 02:55:33 PDT
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) 2012-04-16 02:56:19 PDT
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] 2012-04-16 11:43:29 PDT
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] 2012-04-16 12:17:36 PDT
(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] 2012-04-16 13:01:11 PDT
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) 2012-04-16 13:59:38 PDT
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) 2012-08-26 04:31:38 PDT
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] 2012-08-26 12:37:49 PDT
http://hg.mozilla.org/projects/alder/rev/ae0ba897db5c

Note You need to log in before you can comment on or make changes to this bug.