On Wed, Mar 20, 2019 at 10:51 AM Tom Lane <tgl@xxxxxxxxxxxxx> wrote: > Thomas Munro <thomas.munro@xxxxxxxxx> writes: > > Even though I can't reproduce the problem myself, I'm quite keen to go > > ahead and push the patch I proposed for v12 anyway, and close this > > case. Otherwise this problem could just keep coming back until > > libldap.so is eventually entirely phased out by all distros. In 2023 > > I want to be working on quantum parallelism or something, not LDAP bug > > reports. Any objections? > > Do we have any clear reason to believe this'd actually fix Mike's problem? > AFAIK the analogy to the old destructor-conflict issue is just a guess, > and we don't really know exactly what is going wrong. Right, we don't know. To learn more about the reported crash I think we'll need Mike to install debug symbols, attach with gdb and make it crash, then show us the output of "bt". More info here: https://wiki.postgresql.org/wiki/Getting_a_stack_trace_of_a_running_PostgreSQL_backend_on_Linux/BSD It'd be nice to be able to rule it out in any future bug reports with these symptoms though, and it's roughly in line with what we see the rest of the open source ecosystem doing about this problem. > It's reasonable to assume that the proposed patch won't cause real issues > on any modern platform, but I'm not sure we can assume that for old ones, > so the whole thing is making me a bit nervous. Still, it's nice > simplification to not have different frontend and backend LDAP libs. Sure, it's possible that some BF animal will fail to link the backend for some reason that requires a bit of investigation and a follow-up patch. Are you thinking of systems not covered by the BF? Unless the server is being built with an extremely small set of configure options enabled, it's almost certainly already linking something that pulls in the platform's threading library (SSL, GSSAPI, XML2, ...). If someone out there is not enabling any of that stuff because their system doesn't like threads, they can use --disable-thread-safety to avoid the effects of this change. > As far as the specifics of the patch go, I don't like that you didn't > adjust any of the comments near pthread_is_threaded_np() usages. Hmm. The comments seemed OK to me without adjustment, is there something specific that bothered you? The errhint about LC_ALL is wrong though, it's macOS-specific. So I think I should change the hint to "On macOS, ...", or I guess make it conditional. -- Thomas Munro https://enterprisedb.com
