Skip to content

Commit

Permalink
Fix crash with PythonAlgorithm registration on windows. Refs #6122
Browse files Browse the repository at this point in the history
There were several issues. First, there were two signal updates happening
at the same time: one from MantidUI and another from
AlgorithmSelectorWidget. Secondly, the AlgorithmSelectorWidget was
receiving a Poco::Notification from a separate thread and not dispatching
it to the GUI thread but instead calling update directly.
  • Loading branch information
martyngigg committed Nov 16, 2012
1 parent 978d95b commit bdc7ae8
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
*WIKI*"""
import sys
from MantidFramework import *
from mantid.simpleapi import *
from mantid.kernel import *
from mantid.api import *
#from mantid.kernel import *
from mantid.simpleapi import *

class QueryFlag:
def isMasked(self, detector, yValue):
Expand Down
9 changes: 9 additions & 0 deletions Code/Mantid/Framework/PythonAPI/src/FrameworkManagerProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <vector>
#include "MantidAPI/IMDHistoWorkspace.h"
#include "MantidAPI/IPeaksWorkspace.h"
#include <Poco/ScopedLock.h>

using Mantid::API::AlgorithmManager;
using Mantid::API::IPeaksWorkspace;
Expand Down Expand Up @@ -534,6 +535,11 @@ bool FrameworkManagerProxy::workspaceExists(const std::string & name) const
return API::AnalysisDataService::Instance().doesExist(name);
}

namespace
{
Poco::Mutex PYALG_REGISTER_MUTEX;
}

/**
* Add a python algorithm to the algorithm factory
* @param pyobj :: The python algorithm object wrapped in a boost object
Expand All @@ -547,6 +553,9 @@ void FrameworkManagerProxy::registerPyAlgorithm(boost::python::object pyobj)
using boost::python::handle;
using boost::python::borrowed;

// Serialize write access to the factory
Poco::ScopedLock<Poco::Mutex> lock(PYALG_REGISTER_MUTEX);

PyObject *classObject = PyObject_GetAttrString(pyobj.ptr(), "__class__");
object classType(handle<>(borrowed(classObject)));
// Takes ownership of instantiator and replaces any existing algorithm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <boost/python/class.hpp>
#include <boost/python/def.hpp>
#include <Poco/ScopedLock.h>

// Python frameobject. This is under the boost includes so that boost will have done the
// include of Python.h which it ensures is done correctly
Expand Down Expand Up @@ -73,14 +74,24 @@ void export_AlgorithmFactory()

namespace
{
// A function to register an algorithm from Python
void registerAlgorithm(boost::python::object obj)
{
using Mantid::PythonInterface::PythonObjectInstantiator;
using Mantid::Kernel::AbstractInstantiator;
using Mantid::PythonInterface::AlgorithmWrapper;
using Mantid::API::Algorithm;
using Mantid::API::IAlgorithm_sptr;

/// Python algorithm registration mutex in anonymous namespace (aka static)
Poco::Mutex PYALG_REGISTER_MUTEX;

/**
* A free function to register an algorithm from Python
* @param obj :: A Python object that should either be a class type derived from PythonAlgorithm
* or an instance of a class type derived from PythonAlgorithm
*/
void registerAlgorithm(boost::python::object obj)
{
Poco::ScopedLock<Poco::Mutex> lock(PYALG_REGISTER_MUTEX);

static PyObject * const pyAlgClass = (PyObject*)converter::registered<AlgorithmWrapper>::converters.to_python_target_type();
// obj could be or instance/class, check instance first
PyObject *classObject(NULL);
Expand Down
10 changes: 0 additions & 10 deletions Code/Mantid/MantidPlot/src/Mantid/MantidUI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ m_finishedLoadDAEObserver(*this, &MantidUI::handleLoadDAEFinishedNotification),
m_replaceObserver(*this,&MantidUI::handleReplaceWorkspace),
m_deleteObserver(*this,&MantidUI::handleDeleteWorkspace),
m_clearADSObserver(*this,&MantidUI::handleClearADS),
m_algUpdatesObserver(*this, &MantidUI::handleAlgorithmFactoryUpdates),
m_renameObserver(*this,&MantidUI::handleRenameWorkspace),
m_groupworkspacesObserver(*this,&MantidUI::handleGroupWorkspaces),
m_ungroupworkspaceObserver(*this,&MantidUI::handleUnGroupWorkspace),
Expand Down Expand Up @@ -194,10 +193,6 @@ void MantidUI::init()
showCritical("The curve fitting plugin is missing");
}

//connect the signal from the algorithm factory to monitor updates
connect(this, SIGNAL(algorithms_updated()), m_exploreAlgorithms, SLOT(update()));
Mantid::API::AlgorithmFactory::Instance().notificationCenter.addObserver(m_algUpdatesObserver);

}

/// Slot: Receives a new X range from a FitPropertyBrowser and re-emits it.
Expand Down Expand Up @@ -293,7 +288,6 @@ MantidUI::~MantidUI()
Mantid::API::AnalysisDataService::Instance().notificationCenter.removeObserver(m_replaceObserver);
Mantid::API::AnalysisDataService::Instance().notificationCenter.removeObserver(m_deleteObserver);
Mantid::API::AnalysisDataService::Instance().notificationCenter.removeObserver(m_clearADSObserver);
Mantid::API::AnalysisDataService::Instance().notificationCenter.removeObserver(m_algUpdatesObserver);
}

void MantidUI::saveSettings() const
Expand Down Expand Up @@ -1779,10 +1773,6 @@ void MantidUI::handleClearADS(Mantid::API::ClearADSNotification_ptr)
emit workspaces_cleared();
}

void MantidUI::handleAlgorithmFactoryUpdates(Mantid::API::AlgorithmFactoryUpdateNotification_ptr)
{
emit algorithms_updated();
}
void MantidUI::handleRenameWorkspace(Mantid::API::WorkspaceRenameNotification_ptr pNf)
{
emit workspace_renamed(QString::fromStdString(pNf->object_name()), QString::fromStdString(pNf->new_objectname()));
Expand Down
4 changes: 0 additions & 4 deletions Code/Mantid/MantidPlot/src/Mantid/MantidUI.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ public slots:
void workspace_replaced(const QString &, Mantid::API::Workspace_sptr);
void workspace_removed(const QString &);
void workspaces_cleared();
void algorithms_updated();
void workspace_renamed(const QString &, const QString &);
void workspaces_grouped(const QStringList&);
void workspace_ungrouped(const QString&, Mantid::API::Workspace_sptr);
Expand Down Expand Up @@ -457,9 +456,6 @@ public slots:
void handleClearADS(Mantid::API::ClearADSNotification_ptr pNf);
Poco::NObserver<MantidUI, Mantid::API::ClearADSNotification> m_clearADSObserver;

void handleAlgorithmFactoryUpdates(Mantid::API::AlgorithmFactoryUpdateNotification_ptr pNf);
Poco::NObserver<MantidUI, Mantid::API::AlgorithmFactoryUpdateNotification> m_algUpdatesObserver;

//handles rename workspace notification
void handleRenameWorkspace(Mantid::API::WorkspaceRenameNotification_ptr pNf);
Poco::NObserver<MantidUI, Mantid::API::WorkspaceRenameNotification> m_renameObserver;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ namespace MantidWidgets
void findAlgTextChanged(const QString& text);
void treeSelectionChanged();

signals:
signals:
void algorithmFactoryUpdateReceived();
void executeAlgorithm(const QString &, int);
void algorithmSelectionChanged(const QString &, int);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ namespace MantidWidgets
layout->addLayout(buttonLayout);
layout->addWidget(m_tree);

// The poco notification will be dispacted from the callers thread but we need to
// make sure the updates to the widgets happen on the GUI thread. Dispatching
// through a Qt signal will make sure it is in the correct thread.
AlgorithmFactory::Instance().notificationCenter.addObserver(m_updateObserver);
connect(this, SIGNAL(algorithmFactoryUpdateReceived()), this, SLOT(update()));
}

//----------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -178,10 +182,7 @@ namespace MantidWidgets
void AlgorithmSelectorWidget::
handleAlgorithmFactoryUpdate(Mantid::API::AlgorithmFactoryUpdateNotification_ptr)
{
if(!m_updateInProgress)
{
this->update();
}
emit algorithmFactoryUpdateReceived();
}

//============================================================================
Expand Down

0 comments on commit bdc7ae8

Please sign in to comment.