[Aptitude-devel] Acquire considered non-threadsafe
Julian Andres Klode
jak at debian.org
Mon Nov 23 23:00:21 UTC 2009
On Mon, Nov 23, 2009 at 07:41:42AM -0800, Daniel Burrows wrote:
> On Thu, Nov 19, 2009 at 06:09:42PM +0100, Julian Andres Klode <jak at debian.org> was heard to say:
> > On Thu, Nov 19, 2009 at 04:31:13AM -0800, Daniel Burrows wrote:
> > > Also, apt2? When did that happen?
> > APT2 will be a package management library + applications written in Vala,
> > but at the moment it's just a few parsers and a simple acquire system. And
> > it's intented to be cross-distro, just like PackageKit. I just had no better
> > name when I started it. It's currently MIT-licensed, but may switch to the
> > LGPL-2.1+ soon (as this is what all its dependencies like glib use).
>
> Hm. I'm pretty skeptical about writing a system tool like this in a
> high-level language. Both from the point of view of dependency
> fragility and because you want it to be easy to access the functionality
> from other languages. In fact, I've occasionally wondered if it makes
> sense for the public API of an apt successor to be pure C (even if the
> implementation is C++ behind the scenes). C++ ABIs are obnoxiously
> fragile and poorly defined, for one thing. (although very convenient if
> you're writing in C++)
Vala compiles to C, and provides a C-API comparable to other GObject libraries
such as GTK+. You get a C library with a header file and everything, and you
can write applications using the library in C. For now, dependencies are
libglib2.0-0
libgee1 (container classes)
libsoup2.4-1 (HTTP/HTTPS acquire part)
The latter one could be replaced with a limited http downloader for embedded
systems; which only depends on libglib2.0-0.
>
> > >
> > > > (f) storing errno inside the error handling objects,
> > > > for bindings (nice to have errno).
> > >
> > > Please, please, kill the global error queue. This is a disaster from
> > > a threadsafety point of view, but also just from a general design
> > > perspective. There's way too much code that does
> > >
> > > if(!_error->PendingError())
> > > fail();
> > >
> > > when the error could have been caused by something entirely unrelated
> > > to what this code is interested in.
> > >
> > > Put error-handling oun either a return-code basis or an exception
> > > basis, but not this wird intermediate thing where random errors cause
> > > failure. If you want to know about inner errors, use a logging
> > > framework (I prefer log4cxx).
> > As far as I can tell from the documentation, the APT error handling should
> > only be used for boolean functions. If the function has an error, it pushes
> > a message to the error stack and returns false. If we used it this way only,
> > we would have no multi-threading problems.
>
> Many uses of PendingError() in apt-pkg need to be fixed then. The
> problem is, when it's used this way, you can't tell which errors the
> code is actually supposed to detect, which makes it tricky to pull the
> call out safely (you'll end up changing behavior). Would you be OK with
> a patch that eliminated these usages anyway? What if I also changed the
> code to use return codes in places where it's obvious which error(s)
> is/are being checked?
>
> Grep shows me 38 uses of PendingError() in apt-pkg. At a quick
> glance, it looks like most of those are problematic. I didn't look
> closely enough to determine how hard each one will be to fix.
>
> I would like to emphasize that this is not just a multi-threading
> problem; any time that the error queue might be in an unknown state (for
> instance, after pkgAcquire::Pulse() is invoked), you could see spurious
> failures.
OK.
>
> > Exceptions are a bit tricky in C++ as they can happen unexpectedly. Google's
> > C++ style guide says no; so we should probably also say no.
>
> Do we work for Google? I wanna get paid in that case :P
>
Just a reference.
> OTOH, most of the Boost libraries are compile-time-only, so aside from
> bootstrapping this isn't necessarily a problem (assuming we don't use
> iostreams or anything like that); policy explicitly excludes build-time
> dependencies from priority rules. On the first hand, a good shared_ptr
> implementation, which is in C++0x IIRC, would be a nice starting point;
> most of the other stuff I use from Boost is more special-purpose. And
> C++0x has some other really nice (and badly needed) stuff in it.
shared_ptr is also available in std::tr1::shared_ptr, at least in current
GCC versions. We could even require a GCC with the C++0x stuff we need
an build with --std=c++0x.
>
> > Some kind of a roadmap:
> >
> > * identify stuff we don't use anymore and remove it, eg.
> > apt-pkg/vendor*, apt-inst/database*, apt-inst/deb/dpkgdb*.
> > * drop compatibility for GCC < 4.3, and other stuff older
> > then what is included in Debian Lenny.
> > * rework the buildsystem using CMake
>
> (ABI breaking stuff)
> * Write a more organized download system. (see below)
>
> Maybe also figure out a style guide for C++ going forwards (e.g.,
> recommend not using exceptions, using smart pointers, using STL
> containers, etc).
Or copy Google's style guide and remove the stuff we don't like; and
adjust it to our coding standards. Their style guide is very complete,
and thus a good base.
> As far as the download system goes, I'd prefer to see an explicit
> two-level design, where the lower level downloads URIs without caring
> what they "mean", and the upper level manages "jobs" that download
> multiple URIs (such as "update some packages"), using the lower level
> object to do the actual download. This would make it a lot easier for
> code linked against apt to make use of apt's nice download functionality
> without having to jump through hoops just to download a URI. (one of my
> pet peeves)
Yes.
>
> In aptitude, I have built an internal API that provides a low-level
> URI downloader running in a background thread. Currently it's based on
> Acquire, but with a little work it could replace Acquire instead
> (at first glance, the Acquire main loop and the download queue management
> would need to be incorporated). I think it's a good example of the
> simple interface I'm thinking about, anyway:
>
> http://hg.debian.org/hg/aptitude/head/file/b8a7fc61b13c/src/generic/apt/download_queue.h
I'm missing a few parts like a target filename, but maybe this is defined
somewhere else?
--
Julian Andres Klode - Debian Developer, Ubuntu Member
See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.
More information about the Aptitude-devel
mailing list