Patricia,
Based on Damjan's finding that profile files are read by the application and
that some are created during setup, with others copied in from the setup, it is
settled that the code is used in current distributions and is also available to
extensions.
I recommend that we catch our breath over the weekend, take a fresh look at
what we know, and then make that smallest repair that will work.
I think that will include the following:
1. Moving the bogus unlock in osl_closeProfile to immediately after bIsValid
is set false at line 300. We also need to confirm that any other of the
exported procedures that synchronize on the mutex immediately release on
finding bIsValid set false.
2. Verifying whether the two calls on acquireProfile depend on the mutex and
whether any returned pointer is to a locked struct or not.
3. Depending on (2), determine whether or not it is appropriate to have an
unconditional unlock at the end of the osl_closeProfile or not. Repair that
aspect accordingly.
4. Also determine whether the top *pProfile structure can be deleted or not.
Should it be left to guard against the possibility that a subsequent operation
on some thread will attempt to use the mutex and check the bIsValid flag? That
is the safe approach.
4.1 Find out if osl_openProfile always returns a fresh *oslProfile
structure whenever it succeeds.
4.2 If so, the C++ wrapper destructor, ~Profile, can always delete the
*profile in its state since it is not shared.
4.3 Any remaining "memory leak" should be cleaned-up at process shutdown.
I have the makings of a nano-branch with only the relevant files. We can use
that to chew up and annotate the relevant source without munging the code base.
I'll put that on the SVN shortly and we can add our notes and confirmed
preconditions before settling on the repair that goes to trunk.
I would defer any extensive threading analysis and work on more pressing
maintenance issues first, especially since the UDK is snarled up in this. It's
worthwhile but other priorities deserve our attention.
- Dennis
> -----Original Message-----
> From: Patricia Shanahan [mailto:p...@acm.org]
> Sent: Friday, March 4, 2016 05:23
> To: dev@openoffice.apache.org
> Subject: Re: Profile.c bugs (was RE: Some thoughts on the learning
> curve)
>
>
>
> On 3/4/2016 4:40 AM, Damjan Jovanovic wrote:
> > On Fri, Mar 4, 2016 at 11:59 AM, Patricia Shanahan <p...@acm.org>
> wrote:
> >> On 3/4/2016 12:54 AM, Damjan Jovanovic wrote:
> >>>
> >>> ELF binaries (Linux, *BSD) fundamentally use one of the worst ideas
> of
> >>> all time: symbols are process scoped (unlike on Windows and MacOS
> >>> where they're library scoped), meaning that symbols with the same
> name
> >>> can clash even if in different libraries loaded through arbitrary
> >>> layers of indirection. For example, compiling AOO with GCC on recent
> >>> FreeBSD easily crashes AOO because both GCC's and Clang's C++
> runtime
> >>> libraries are loaded, their symbols clash with each other, and their
> >>> ABIs are not fully compatible -> memory corruption -> undebuggable
> >>> crash. Symbol map files are supposed to work around the problem
> >>> because the UDK_3_0_0 becomes part of the symbol name and makes it
> >>> more globally unique.
> >>>
> >>> I don't know if UDK_3_0_0 is correct since we are on AOO version 4
> now
> >>> (or do we version UDK differently?).
> >>
> >>
> >> Embedding version numbers in source code should be done very
> sparingly,
> >> because it grows old fast.
> >>
> >>>
> >>> Also UNO is all good in theory, but in practice plugins can always
> use
> >>> run-time dynamic linking to load any library and call any exported
> >>> function (which the profile functions are) - even when running in a
> >>> separate process, or written a different language (eg. JNA/BridJ for
> >>> Java). Whether they actually do this in practice is the more
> important
> >>> question.
> >>
> >>
> >> I have a possibly pathetic hope that the 14 year deprecation of the
> >> functions may have discouraged their direct use in plugins.
> >>
> >>
> >
> > main/sal/rtl/source/bootstrap.cxx uses osl::Profile().readString() in
> > 1 place. osl::Profile found in main/sal/inc/osl/profile.hxx is a thin
> > C++ wrapper around the C API. That seems to be the extent of the usage
> > of the profile API in SVN trunk. I can't remember where else I found
> > them, but on
> https://wiki.openoffice.org/wiki/Documentation/DevGuide/WritingUNO/Manua
> l_Component_Installation
> > there is a brief mention of these ini/rc files that I suspect are
> > parsed by the profile API. Examples are in the program/ directory of
> > the AOO installation (there is quite a few, all ending in ".ini" on
> > Windows and "rc" elsewhere, eg. uno.ini/unorc).
> >
> > As for locking, all platforms can and do lock the file against access
> > from other processes (LockFileEx on Windows, fcntl+F_SETLKW on *nix)
> > though on *nix it is only *advisory* locking. The pthread API would
> > only be useful to ensure thread safety within the process. I presume
> > "bootstrapping" only happens once per process, so it can't really be
> > multi threaded anyway?
>
> My current theory is that the "unx" profile.c pthread_mutex uses are
> bogus, and should be removed. It would be better to keep profile
> operations single threaded at the top level, not engage in fine grained
> locking of Profile structs.
>
> I suspect that an attempt to make the Profile code thread-safe was in
> process in 2000 at the time of the snapshot of StarOffice code that
> started OpenOffice. The Sun developers would have been working mainly on
> Solaris, so the "unx" version would be the trailblazer.
>
> Patricia
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org
> For additional commands, e-mail: dev-h...@openoffice.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@openoffice.apache.org
For additional commands, e-mail: dev-h...@openoffice.apache.org
