make internal libevent work on BSDs

RESOLVED FIXED in Firefox 22

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Jan Beich, Assigned: Jan Beich)

Tracking

Trunk
mozilla23
All
FreeBSD
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox21 unaffected, firefox22 fixed, firefox23 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
With libevent-2.0.21 imported it's now possible to build Mozilla's
copy of libevent on *BSD systems without additional patches. Only the
config header is missing.

  ipc/chromium/src/third_party/libevent/buffer.c:28:10: fatal error:
	'event2/event-config.h' file not found
  #include "event2/event-config.h"
	   ^
  1 error generated.
  gmake[1]: *** [buffer.o] Error 1
(Assignee)

Comment 1

5 years ago
Created attachment 739965 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

It seems the only difference is sendfile(2) being available on
DragonFly and FreeBSD but used only on FreeBSD (upstream bug?).
Attachment #739965 - Flags: review?(benjamin)
(Assignee)

Comment 2

5 years ago
Created attachment 739966 [details] [diff] [review]
kqueue/kevent visibility for gcc_hidden.h
Attachment #739966 - Flags: review?(benjamin)
I'll have to check this, since with 1.4.7 i always had to build with external libevent (i have this in my mozconfig) :

# fails to find sys/epoll.h header
ac_add_options --with-system-libevent=/usr/

Updated

5 years ago
Attachment #739966 - Flags: review?(benjamin) → review?(mh+mozilla)

Comment 4

5 years ago
Comment on attachment 739965 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

Going to delegate this review to Gaston, since I don't have an opinion about it.
Attachment #739965 - Flags: review?(benjamin) → review?(landry)
Comment on attachment 739965 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

I'm not sure i'm qualified to do the actual review since i'm not the owner of anything in this area, but without the patch the build indeed fails with :

/src/mozilla-central/ipc/chromium/src/third_party/libevent/bufferevent.c:30:10: fatal error: 'event2/event-config.h' file not found
#include "event2/event-config.h"
         ^
/src/mozilla-central/ipc/chromium/src/third_party/libevent/buffer.c:28:10: fatal error: 'event2/event-config.h' file not found
#include "event2/event-config.h"

And with the patch, i can see kqueue.o being built in ipc/chromium and being used by message_pump_libevent.o. No idea how to actually runtime-test this.

As for the defines used in event-config.h, i'll trust you on it - if it comes from netbsd it should be good for us too. It looks similar to the macos one, besides a few additions..
Attachment #739965 - Flags: review?(landry) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 740659 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

keeping r+ for trivial change:

  -ifneq (,$(filter DragonFly FreeBSD,$(OS_TARGET))) # {
  +ifneq (,$(OS_DRAGONFLY)$(OS_FREEBSD)) # {

(In reply to Landry Breuil (:gaston) from comment #5)
> And with the patch, i can see kqueue.o being built in ipc/chromium and being
> used by message_pump_libevent.o. No idea how to actually runtime-test this.

Try syscalls from plugin-container ? It generates lots of kevent(2)
calls with flash plugin here.

  $ ktrace -tc -p $(pgrep plugin-container)
  $ kdump | fgrep kevent

or

  (gdb) bt
  #0  kevent () at kevent.S:3
  #1  0x00000008023a4a4d in kq_dispatch () from dist/bin/libxul.so
  #2  0x00000008023949aa in event_base_loop () from dist/bin/libxul.so
  #3  0x00000008023d06a3 in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) () from dist/bin/libxul.so
  #4  0x00000008023af0dd in MessageLoop::Run() () from dist/bin/libxul.so
  #5  0x00000008023bc767 in base::Thread::ThreadMain() () from dist/bin/libxul.so
  #6  0x00000008023d0c47 in ThreadFunc(void*) () from dist/bin/libxul.so
  #7  0x0000000804cc5413 in thread_start (curthread=0x80ee03400) at lib/libthr/thread/thr_create.c:284
  #8  0x0000000000000000 in ?? ()
Attachment #739965 - Attachment is obsolete: true
(In reply to Jan Beich from comment #6)
> Created attachment 740659 [details] [diff] [review]
> add one config for all bsd (from netbsd x86_64)
> 
> keeping r+ for trivial change:
> 
>   -ifneq (,$(filter DragonFly FreeBSD,$(OS_TARGET))) # {
>   +ifneq (,$(OS_DRAGONFLY)$(OS_FREEBSD)) # {
> 
> (In reply to Landry Breuil (:gaston) from comment #5)
> > And with the patch, i can see kqueue.o being built in ipc/chromium and being
> > used by message_pump_libevent.o. No idea how to actually runtime-test this.
> 
> Try syscalls from plugin-container ? It generates lots of kevent(2)
> calls with flash plugin here.

flash plugin on OpenBSD ? you must be joking :)

More seriously, i'll give it a shot with gecko-mediaplayer which is apparently run inside plugin-container.
Comment on attachment 740659 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

It seems to work just fine with gecko-mediaplayer. So definitely r+ from me - awaiting mike's review to land this.
Attachment #740659 - Flags: review+
Attachment #739966 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 10

5 years ago
Comment on attachment 740659 [details] [diff] [review]
add one config for all bsd (from netbsd x86_64)

[Approval Request Comment]
Bug caused by: since chromiumipc import (longstanding)
User impact if declined: having to specify --with-system-libevent on bsd systems
Risk to taking this patch (and alternatives if risky): none
Attachment #740659 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 11

5 years ago
Comment on attachment 739966 [details] [diff] [review]
kqueue/kevent visibility for gcc_hidden.h

OS X uses kqueue in sys/event.h but due to buggy libstdc++-4.2
visibility pragmas aren't used there. You have to use modern
libstdc++ (GPLv3) or libc++ (clang only) in order to reproduce.

[Approval Request Comment]
User impact if declined: having to force fallback to -fvisibility=hidden
Attachment #739966 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5dc7a73fee05
https://hg.mozilla.org/mozilla-central/rev/3fb99a9ab106
Assignee: nobody → jbeich
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 739966 [details] [diff] [review]
kqueue/kevent visibility for gcc_hidden.h

In support of downstream
Attachment #739966 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #740659 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/22b2c9c14a91
https://hg.mozilla.org/releases/mozilla-aurora/rev/3989318b0b72
status-firefox21: --- → unaffected
status-firefox22: --- → fixed
status-firefox23: --- → fixed
(Assignee)

Updated

4 years ago
Blocks: 969932
You need to log in before you can comment on or make changes to this bug.