Skip to content

Commit

Permalink
Fixed segfault in iteration over storage plugin names
Browse files Browse the repository at this point in the history
Also changed plugin help to be a class method.
  • Loading branch information
jesper-friis committed Aug 20, 2023
1 parent 122ae41 commit 89da0c6
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 22 deletions.
34 changes: 31 additions & 3 deletions bindings/python/dlite-storage-python.i
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# Override default __init__()
def __init__(self, driver_or_url, location=None, options=None):
loc = str(location) if location else None
_dlite.Instance_swiginit(self, _dlite.new_Storage(
_dlite.Storage_swiginit(self, _dlite.new_Storage(
driver_or_url=driver_or_url, location=loc, options=options))

def __enter__(self):
Expand Down Expand Up @@ -41,10 +41,25 @@
)

@classmethod
def create_from_url(cls, url):
def from_url(cls, url):
"""Create a new storage from `url`."""
return cls(url)

@classmethod
def load_plugins(cls):
"""Load all storage plugins."""
_dlite._load_all_storage_plugins()

@classmethod
def unload_plugin(cls, name):
"""Unload storage plugin with this name."""
_dlite._unload_storage_plugin(str(name))

@classmethod
def plugin_help(cls, name):
"""Return documentation of storage plogin with this name."""
return _dlite._storage_plugin_help(name)

def instances(self, pattern=None):
"""Returns an iterator over all instances in storage whos
metadata URI matches `pattern`."""
Expand All @@ -64,6 +79,7 @@

driver = property(get_driver,
doc='Name of driver associated with this storage')

%}
}

Expand All @@ -77,12 +93,24 @@
%}
}


%extend StoragePluginIter {
%pythoncode %{
# Override default __init__()
def __init__(self):
"""Iterates over loaded storage plugins."""
_dlite.StoragePluginIter_swiginit(
self, _dlite.new_StoragePluginIter())
# Keep a reference to self, such that it is not garbage-collected
# before end of iterations
if not hasattr(_dlite, "_storage_plugin_iters"):
_dlite._storage_plugin_iters = {}
_dlite._storage_plugin_iters[id(self.iter)] = self

def __next__(self):
name = self.next()
if not name:
# Delete reference to iterator object stored away in __init__()
del _dlite._storage_plugin_iters[id(self.this)]
raise StopIteration()
return name
%}
Expand Down
22 changes: 18 additions & 4 deletions bindings/python/dlite-storage.i
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,17 @@

%{
#include "dlite.h"
#include "dlite-errors.h"
#include "dlite-storage.h"
#include "dlite-storage-plugins.h"

char *_storage_plugin_help(const char *name) {
const DLiteStoragePlugin *api = dlite_storage_plugin_get(name);
if (api->help) return api->help(api);
return dlite_err(dliteUnsupportedError,
"\"%s\" storage does not support help", name), NULL;
}

%}


Expand Down Expand Up @@ -68,8 +77,8 @@ enum _DLiteIDFlag {
%feature("docstring", "\
Represents a data storage.

Parameters
----------
Arguments
---------
driver_or_url : string
Name of driver used to connect to the storage or, if `location` is not
given, the URL to the storage:
Expand Down Expand Up @@ -195,9 +204,9 @@ Iterates over loaded storage plugins.
DLiteStoragePluginIter *iter;
};
%}
%feature("docstring", "") new_StoragePluginIter;
%extend StoragePluginIter {
StoragePluginIter(void) {
//dlite_storage_plugin_load_all();
DLiteStoragePluginIter *iter = dlite_storage_plugin_iter_create();
return (struct StoragePluginIter *)iter;
}
Expand All @@ -218,9 +227,14 @@ Iterates over loaded storage plugins.
}


%rename(storage_unload) dlite_storage_plugin_unload;
%rename(_load_all_storage_plugins) dlite_storage_plugin_load_all;
int dlite_storage_plugin_load_all();

%rename(_unload_storage_plugin) dlite_storage_plugin_unload;
int dlite_storage_plugin_unload(const char *name);

char *_storage_plugin_help(const char *name);


/* -----------------------------------
* Target language-spesific extensions
Expand Down
1 change: 1 addition & 0 deletions bindings/python/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ set(tests
test_misc
test_python_storage
test_storage
test_storage_plugins
test_paths
test_utils
test_global_dlite_state
Expand Down
25 changes: 25 additions & 0 deletions bindings/python/tests/test_storage_plugins.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import dlite


# Initially no storage plugins are loaded
assert list(dlite.StoragePluginIter()) == []

# Storage plugins can be loaded
dlite.Storage.load_plugins()

# Now plugins are loaded
plugins = set(dlite.StoragePluginIter())
assert plugins
assert "json" in plugins

# Plugins can be iterated over
print("Storage plugins")
plugins2 = set()
for name in dlite.StoragePluginIter():
print(" -", name)
plugins2.add(name)
assert plugins2 == plugins

# Unload json plugin
dlite.Storage.unload_plugin("json")
assert "json" not in set(dlite.StoragePluginIter())
2 changes: 1 addition & 1 deletion src/dlite-storage-plugins.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ typedef int (*Flush)(DLiteStorage *s);
Returns a malloc'ed string with plugin documentation or NULL on error.
Optional.
*/
typedef char *(*Help)(DLiteStorage *s);
typedef char *(*Help)(const DLiteStoragePlugin *api);

/** @} */

Expand Down
2 changes: 1 addition & 1 deletion src/dlite-storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ int dlite_storage_delete(DLiteStorage *s, const char *id)
*/
char *dlite_storage_help(DLiteStorage *s)
{
if (s->api->help) return s->api->help(s);
if (s->api->help) return s->api->help(s->api);
return err(dliteUnsupportedError, "storage does not support help: %s",
s->api->name), NULL;
}
Expand Down
10 changes: 0 additions & 10 deletions src/utils/plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,6 @@ PluginInfo *plugin_info_create(const char *kind, const char *symbol,
map_init(&info->pluginpaths);
map_init(&info->apis);

printf("\n");
const char *key;
map_iter_t iter = map_iter(&info->apis);
printf("*** iter:\n");
while ((key = map_next(&info->apis, &iter)))
printf("*** key='%s'\n", key);
printf("*** iterdone:\n");

return info;
}

Expand Down Expand Up @@ -422,7 +414,6 @@ char **plugin_names(const PluginInfo *info)
*/
void plugin_api_iter_init(PluginIter *iter, const PluginInfo *info)
{
printf("*** init iter...\n");
memset(iter, 0, sizeof(PluginIter));
iter->info = info;
iter->miter = map_iter(&info->apis);
Expand All @@ -440,7 +431,6 @@ const PluginAPI *plugin_api_iter_next(PluginIter *iter)
PluginAPI **p, *api=NULL;
PluginInfo *info = (PluginInfo *)iter->info;
const char *name = map_next(&info->apis, &iter->miter);
printf(" -- '%s'\n", name);
if (!name) return NULL;
if (!(p = map_get(&info->apis, name)) || !(api = *p))
fatal(1, "failed to get api: %s", name);
Expand Down
6 changes: 3 additions & 3 deletions storages/python/dlite-plugins-python.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,17 @@ int flusher(DLiteStorage *s)
It combines the class documentation with the documentation of the open()
method.
*/
char *helper(DLiteStorage *s)
char *helper(const DLiteStoragePlugin *api)
{
PyObject *v=NULL, *pyclassdoc=NULL, *open=NULL, *pyopendoc=NULL;
PyObject *class = (PyObject *)s->api->data;
PyObject *class = (PyObject *)api->data;
const char *classname, *classdoc=NULL, *opendoc=NULL;
char *doc=NULL;
Py_ssize_t n=0, clen=0, olen=0, i, newlines=0;

dlite_errclr();
if (!(classname = dlite_pyembed_classname(class)))
dlite_warnx("cannot get class name for storage plugin '%s'", s->api->name);
dlite_warnx("cannot get class name for storage plugin '%s'", api->name);

if (PyObject_HasAttrString(class, "__doc__")) {
if (!(pyclassdoc = PyObject_GetAttrString(class, "__doc__")))
Expand Down

0 comments on commit 89da0c6

Please sign in to comment.