Re: Server crash when using bgw_main for a dynamic bgworker

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Server crash when using bgw_main for a dynamic bgworker
Date: 2013-08-16 18:20:08
Message-ID: CA+TgmoboADqkQVSodfiPd=+3EbZtqaYQPO2v8igg1OovQ2YqiA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 12, 2013 at 1:26 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> While playing a bit with background workers (commit 527ea66), I found that
> setting bgw_main for a dynamic bgworker, as well as bgw_library_name and
> bgw_library_name, crashes to server if the library defining the function
> defined in bgw_main is not loaded.

Yep. As the documentation says: "bgw_main may be NULL; in that case,
bgw_library_name and bgw_function_name will be used to determine the
entrypoint. This is useful for background workers launched after
postmaster startup, where the postmaster does not have the requisite
library loaded."

http://www.postgresql.org/docs/devel/static/bgworker.html

> Wouldn't be clearer for the user to add a new flag in
> BackgroundWorker:bgworker.h to define a class of bgworker? Or at least
> specify clearly in the docs just to never set bgw_main if the library is not
> loaded previously using for example shared_preload_libraries?

I don't think adding a flag really eliminates any possibility for user
confusion that doesn't already exist as things stand. However,
changing the documentation might make sense. The key point is that if
the function must be loaded into both the registering backend and the
worker backend, and it must also be loaded into both backends *at the
same address*. Anything that's loaded later than
shared_preload_libraries isn't likely to be soon enough, and even if
you do load the library at shared_preload_libraries time, I'm not
clear that it's going to work reliably in an EXEC_BACKEND environment.
What if the OS uses ASLR and puts the shared library at different
addresses in different child processes? It'll crash, that's what.

It's almost tempting to tell people "don't use bgw_main", but I think
that might be going overboard. This is expert-level hackery; experts
don't usually like being told what to do without being told why they
must.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-08-16 18:21:44 Re: [PATCH] pg_sleep(interval)
Previous Message Alvaro Herrera 2013-08-16 17:42:17 Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])