Mailing-List: contact dev-help@openoffice.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@openoffice.apache.org
Reply-To: <dennis.hamilton@acm.org>
From: "Dennis E. Hamilton" <dennis.hamilton@acm.org>
To: <dev@openoffice.apache.org>
References: <003c01d17323$b021a6a0$1064f3e0$@acm.org>
 <56D72FF5.9090307@acm.org> <00ab01d174bb$a5d0dc90$f17295b0$@acm.org>
 <005c01d17580$ecab0670$c6011350$@acm.org> <56D8BCC3.4070008@acm.org>
 <00d801d175b1$4fff2f00$effd8d00$@acm.org> <56D8E840.8050205@acm.org>
 <CAJm2B-k1K34tmkRdiO+ZNXTuMLVGwrgsmSTX7hqKOZT-t=dZiQ@mail.gmail.com>
 <56D95C86.5010602@acm.org>
 <CAJm2B-=yxqGbxpa-pWpC_Txsk0Pw=kxHzywhKXjorF3xfAqtaw@mail.gmail.com>
 <56D98C1F.5060200@acm.org>
In-Reply-To: <56D98C1F.5060200@acm.org>
Subject: RE: Profile.c bugs (was RE: Some thoughts on the learning curve)
Date: Fri, 4 Mar 2016 09:39:41 -0800
Organization: NuovoDoc
Message-ID: <004a01d1763c$d5bd66d0$81383470$@acm.org>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: 
 AQEYt3sDAegnni14uEJXPSBAluhsHgFsRKnjAf3TzfQBRz+IPQJuHYp2AmvVPVsB4U/a0gCjG9fEAn6mK+gCyjTBCAGUI/28oCOZLtA=
Content-Language: en-us

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:pats@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)
>=20
>=20
>=20
> On 3/4/2016 4:40 AM, Damjan Jovanovic wrote:
> > On Fri, Mar 4, 2016 at 11:59 AM, Patricia Shanahan <pats@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?
>=20
> 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.
>=20
> 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.
>=20
> Patricia
>=20
>=20
>=20
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
> For additional commands, e-mail: dev-help@openoffice.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@openoffice.apache.org
For additional commands, e-mail: dev-help@openoffice.apache.org