[Debichem-devel] Bug#862517: python-openbabel: Cannot 'import pybel' reliably

[Stuart Prescott]

Package: python-openbabel
Version: 2.3.2+dfsg-2.4+b2
Severity: serious
Tags: patch
Justification: package is unusable

Dear Maintainer,

There is an error in string handling within the openbabel library that is
exposed by trying to enumerate the available plugins. Since pybel tries
to enumerate the plugins, the exception that is raise leads to an
ImportError. Whether this causes an ImportError or not depends on the state
of the interpreter. In my testing about 1/10 times at the python REPL would
fail to import while in the ipython REPL about 1/3 imports would fail.

In [1]: import pybel
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-1-bce5857d9ab2> in <module>()
----> 1 import pybel

/usr/lib/python2.7/dist-packages/pybel.py in <module>()
     85     return [x.split()[0] for x in plugins]
     86 
---> 87 descs = _getpluginnames("descriptors")
     88 """A list of supported descriptors"""
     89 _descdict = _getplugins(ob.OBDescriptor.FindType, descs)

/usr/lib/python2.7/dist-packages/pybel.py in _getpluginnames(ptype)
     83     if sys.platform[:4] == "java":
     84         plugins = [plugins.get(i) for i in range(plugins.size())]
---> 85     return [x.split()[0] for x in plugins]
     86 
     87 descs = _getpluginnames("descriptors")

IndexError: list index out of range


I assume other bindings to the library would also have this problem since it
is a bug in the C++ code not in the python code.

Attached patch (ready to be dropped into debian/patches/ and tested locally)
adapted from

  https://github.com/openbabel/openbabel/pull/62

cheers
Stuart



-- System Information:
Debian Release: 9.0
  APT prefers testing-proposed-updates
  APT policy: (550, 'testing-proposed-updates'), (500, 'testing-debug'), (500, 'testing'), (60, 'unstable')
Architecture: amd64
 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_AU.UTF-8, LC_CTYPE=en_AU.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages python-openbabel depends on:
ii  libc6            2.24-10
ii  libgcc1          1:6.3.0-16
ii  libopenbabel4v5  2.3.2+dfsg-2.4+b2
ii  libstdc++6       6.3.0-16
ii  python           2.7.13-2
pn  python:any       <none>

python-openbabel recommends no packages.

python-openbabel suggests no packages.

-- no debconf information
-------------- next part --------------
>From 45abc715002cce182e71f68a6d48d8d43654f8a2 Mon Sep 17 00:00:00 2001
From: Matt Swain <m.swain at me.com>
Date: Wed, 15 Jan 2014 18:39:06 +0000
Subject: [PATCH] Fix garbage plugin strings bug with libc++

With libc++, using OBDefine to load descriptors from plugindefines.txt would result in some plugin IDs and descriptions becoming garbage.

This was because plugins use const char* pointers for their ID and description, which point towards strings stored in a vector by OBDefine. The problem was that as new strings were read from plugindefines.txt and added to the vector, reallocations can occur to expand the vector capacity, invalidating any existing const char* created from the previously added strings using c_str().

This commit fixes this bug by reading in all descriptors from plugindefines.txt before creating the plugin instances. This ensures no changes are made the string vector once a plugin instance has been created.

https://github.com/openbabel/openbabel/pull/62

---
 src/ops/loader.cpp               | 14 ++++++++++++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/ops/loader.cpp b/src/ops/loader.cpp
index 847b638..ecd4ca4 100644
--- a/src/ops/loader.cpp
+++ b/src/ops/loader.cpp
@@ -92,14 +92,24 @@ class OBDefine : public OBLoader
         //Save a copy of textlines so that const char* pointers in new instance point
         //to something which has not been deleted
         _text.push_back(textlines);
-        //Make an instance of the object. It will be deleted in the destructor
-        _instances.push_back(pdef->MakeInstance(_text.back()));
+        // Previously, instances of the plugins were made here:
+        //_instances.push_back(pdef->MakeInstance(_text.back()));
+        // However, subsequent _text.push_back calls can cause vector reallocations, 
+        // which invalidates the const char* pointers in the instances. So instead we
+        // populate _text fully before making the plugin instances.
       }
       else
         obErrorLog.ThrowError(__FUNCTION__, "Failed to make an instance " + textlines[0], obError);
       textlines.clear();
     }
 
+    // Iterate through _text and make instances of the plugins.
+    // They will be deleted in the destructor.
+    vector<vector<string> >::iterator iter;
+    for(iter=_text.begin();iter!=_text.end();++iter) {
+      OBPlugin* pdef = FindDef((*iter)[0].c_str());
+      _instances.push_back(pdef->MakeInstance(*iter));
+    }
 
     // return the locale to the original one
     obLocale.RestoreLocale();