public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Florian Weimer <fweimer@redhat.com>,
	libc-alpha <libc-alpha@sourceware.org>
Subject: Failing fast: Revert "elf: Always call destructors in reverse constructor order (bug 30785)"
Date: Tue, 24 Oct 2023 06:57:38 -0400	[thread overview]
Message-ID: <cf4a8045-c824-6f1c-ffd1-e433aa45ef2c@redhat.com> (raw)

Florian,

I want to thank you for all of the work that you put into the ELF destructor
and constructor ordering changes. Even if we have had to revert the change we 
connected the solution directly to users to learn if it would work.

You used our downstream connections to test the changes in a broader
context with distribution builds and users in that ecosystem (where we saw
some compatibility issues). Along the way there were several bugs to resolve
and overall the implementation is better because of the attempt.

I specifically want to call out the exceptional "Reason for revert" message
which documents exactly why you did the revert.

    Reason for revert:
    
    The commit changes the order of ELF destructor calls too much relative
    to what applications expect or can handle.  In particular, during
    process exit and _dl_fini, after the revert commit, we no longer call
    the destructors of the main program first; that only happens after
    some dlopen'ed objects have been destructed.  This robs applications
    of an opportunity to influence destructor order by calling dlclose
    explicitly from the main program's ELF destructors.  A couple of
    different approaches involving reverse constructor order were tried,
    and none of them worked really well.  It seems we need to keep the
    dependency sorting in _dl_fini.
    
    There is also an ambiguity regarding nested dlopen calls from ELF
    constructors: Should those destructors run before or after the object
    that called dlopen?  Commit 6985865bc3ad5b2314 used reverse order
    of the start of ELF constructor calls for destructors, but arguably
    using completion of constructors is more correct.  However, that alone
    is not sufficient to address application compatibility issues (it
    does not change _dl_fini ordering at all).

Thank you for attempting to address this issue, and even if we failed
and reverted it, the outcome is that we've discovered a lot of other
implicit dependencies.

I would be keenly interested in seeing a post-mortem blog or presentation
or discussion here where you lay out these topics and more details regarding
your thoughts on the ordering and the C++ language requirements, on application
expectations, and what we might do to simplify what we have today.

In my mind these reverts are an excellent example of failing fast:

* Attempting a valuable change that has risk.
  - Simplify ordering.

* Connecting directly to our users to see if it works.
  - Deploy directly into Fedora.

* Changing the direction of the work as required within a release.
  - Revert.
  - Focus on dlclose() issues.

Thank you for working on these difficult problems :-)

-- 
Cheers,
Carlos.


                 reply	other threads:[~2023-10-24 10:57 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf4a8045-c824-6f1c-ffd1-e433aa45ef2c@redhat.com \
    --to=carlos@redhat.com \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).