public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Failing fast: Revert "elf: Always call destructors in reverse constructor order (bug 30785)"
@ 2023-10-24 10:57 Carlos O'Donell
  0 siblings, 0 replies; only message in thread
From: Carlos O'Donell @ 2023-10-24 10:57 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha

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.


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-10-24 10:57 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-24 10:57 Failing fast: Revert "elf: Always call destructors in reverse constructor order (bug 30785)" Carlos O'Donell

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