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

RESOLVED FIXED

Status

()

Core
WebRTC: Signaling
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 614921 [details] [diff] [review]
remove a set of dependencies on non-unique files/queue names in sipcc
(Assignee)

Comment 2

5 years ago
Verified on Fedora 15 that the cprCreateMessageQueue() creates unique queues using ftok("/proc/self"...) when multiple instances are run at once.
Assignee: nobody → rjesup
(Assignee)

Comment 3

5 years ago
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.
Attachment #614921 - Flags: feedback?(snandaku)
Attachment #614921 - Flags: feedback?(ethanhugg)
Attachment #614921 - Flags: feedback?(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 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

5 years ago
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.
(Assignee)

Comment 7

5 years ago
(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()).

Updated

5 years ago
Attachment #614921 - Flags: feedback?(ethanhugg) → feedback+
(Assignee)

Comment 8

5 years ago
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 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.

Updated

5 years ago
QA Contact: jsmith
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.
Attachment #614921 - Flags: feedback?(emannion) → feedback+
(Assignee)

Comment 11

5 years ago
http://hg.mozilla.org/projects/alder/rev/ae0ba897db5c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [qa-]
(Assignee)

Updated

5 years ago
Attachment #614921 - Flags: feedback?(snandaku)
You need to log in before you can comment on or make changes to this bug.