Closed
Bug 1098607
Opened 10 years ago
Closed 10 years ago
ipc/glue/CrossProcessMutex_unimplemented.cpp compile issue
Categories
(Core :: IPC, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: spz, Assigned: jbeich)
References
Details
Attachments
(2 files)
523 bytes,
patch
|
Details | Diff | Splinter Review | |
1.96 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; NetBSD amd64; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20141113094658 Steps to reproduce: I'm on a OS_POSIX system that is neither Linux nor Darwin and tried to compile firefox. I hit a snag compiling ipc/glue/CrossProcessMutex_unimplemented.cpp Actual results: complaints about 0 being incompatible with the CrossProcessMutexHandle type Expected results: a snagless build :) Instead of the patch, the real fix may be to use CrossProcessMutex__posix.cpp for all OS_POSIX systems.
Updated•10 years ago
|
Attachment #8522540 -
Flags: review?(benjamin)
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 1•10 years ago
|
||
Comment on attachment 8522540 [details] [diff] [review] CrossProcessMutex__unimplemented.cpp.patch jbeich touched this most recently, I'm going to ask him for feedback because I don't even really know what this code does.
Attachment #8522540 -
Attachment is patch: true
Attachment #8522540 -
Attachment mime type: text/x-patch → text/plain
Attachment #8522540 -
Flags: review?(benjamin) → review?(jbeich)
PTHREAD_PROCESS_SHARED isn't implemented on any BSD for ipc/glue/CrossProcessMutex_posix.cpp to be usable. One could try using sem_open(3) instead after finding simple tests for CrossProcessMutex to check it works correctly. Bug 1072093 case for OS X 10.6 suggests only progressive-paint may need an actual implementation. http://mail-index.netbsd.org/current-users/2013/11/29/msg023791.html http://lists.freebsd.org/pipermail/freebsd-threads/2014-February/005531.html
Blocks: 1072093
ipc/chromium code is confusing. Do we support platforms without SharedMemoryBasic ? https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=eba07acf108b
Attachment #8523083 -
Flags: review?(benjamin)
Comment on attachment 8523083 [details] [diff] [review] fix Sorry, should have passed to whoever introduced the regression.
Attachment #8523083 -
Flags: review?(benjamin) → review?(bugmail.mozilla)
Comment on attachment 8522540 [details] [diff] [review] CrossProcessMutex__unimplemented.cpp.patch Review of attachment 8522540 [details] [diff] [review]: ----------------------------------------------------------------- Not a peer or know enough about non-POSIX case. Back to Benjamin if he wants to continue. ::: ipc/glue/CrossProcessMutex_unimplemented.cpp.orig @@ +43,2 @@ > NS_RUNTIMEABORT("Cross-process mutices not allowed on this platform - woah! We should've aborted by now!"); > + return result; /* uninitialized, but we abort before getting here anyway */ If the above works on non-POSIX systems as well because SharedMemoryBasic exists then uintptr_t in CrossProcessMutex.h can be removed: #if defined(OS_WIN) typedef HANDLE CrossProcessMutexHandle; #else typedef mozilla::ipc::SharedMemoryBasic::Handle CrossProcessMutexHandle; #endif
Attachment #8522540 -
Flags: review?(jbeich) → review?(benjamin)
Comment 6•10 years ago
|
||
Comment on attachment 8523083 [details] [diff] [review] fix Review of attachment 8523083 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me, thanks!
Attachment #8523083 -
Flags: review?(bugmail.mozilla) → review+
Comment 7•10 years ago
|
||
Stumbled upon the same issue on OpenBSD, patch fixes it, so pushed to m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/2c130981cdab
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2c130981cdab
Assignee: nobody → jbeich
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 9•9 years ago
|
||
Comment on attachment 8522540 [details] [diff] [review] CrossProcessMutex__unimplemented.cpp.patch This patch isn't necessary any more, right?
Attachment #8522540 -
Flags: review?(benjamin)
Reporter | ||
Comment 10•9 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #9) > Comment on attachment 8522540 [details] [diff] [review] > CrossProcessMutex__unimplemented.cpp.patch > > This patch isn't necessary any more, right? correct
You need to log in
before you can comment on or make changes to this bug.
Description
•