864013 - make internal libevent work on BSDs

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

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

Comment 1

[Jan Beich]

Attachment: 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?).

Comment 2

[Jan Beich]

Attachment: kqueue/kevent visibility for gcc_hidden.h

Comment 3

[Landry Breuil (:gaston)]

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/

Comment 4

[Benjamin Smedberg]

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.

Comment 5

[Landry Breuil (:gaston)]

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..

Comment 6

[Jan Beich]

Attachment: 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 ?? ()

Comment 7

[Landry Breuil (:gaston)]

(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 8

[Landry Breuil (:gaston)]

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.

Comment 9

[Landry Breuil (:gaston)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5dc7a73fee05
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fb99a9ab106

Comment 10

[Jan Beich]

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

Comment 11

[Jan Beich]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5dc7a73fee05
https://hg.mozilla.org/mozilla-central/rev/3fb99a9ab106

Comment 13

[Alex Keybl [:akeybl]]

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

In support of downstream

Comment 14

[Landry Breuil (:gaston)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22b2c9c14a91
https://hg.mozilla.org/releases/mozilla-aurora/rev/3989318b0b72