Bug 2360 - Scripting: "Message:" command may crash when using Nyquist with Python

This commit is contained in:
Leland Lucius 2020-04-16 22:14:45 -05:00
parent 9827d4a753
commit a6d0b3f902
9 changed files with 89 additions and 186 deletions

View File

@ -32,7 +32,6 @@ i.e. an alternative to the usual interface, for Audacity.
#include "PluginManager.h"
#include "commands/ScriptCommandRelay.h"
#include <NonGuiThread.h> // header from libwidgetextra
#include "audacity/PluginInterface.h"
@ -362,9 +361,7 @@ void ModuleManager::Initialize(CommandHandler &cmdHandler)
// After loading all the modules, we may have a registered scripting function.
if(scriptFn)
{
ScriptCommandRelay::SetCommandHandler(cmdHandler);
ScriptCommandRelay::SetRegScriptServerFunc(scriptFn);
NonGuiThread::StartChild(&ScriptCommandRelay::Run);
ScriptCommandRelay::StartScriptServer(scriptFn);
}
}

View File

@ -165,7 +165,7 @@ bool ApplyAndSendResponse::Apply()
{
response += wxT("Failed!");
}
mCtx->Status(response);
mCtx->Status(response, true);
return result;
}

View File

@ -53,11 +53,6 @@ bool CommandBuilder::WasValid()
return mValid;
}
const wxString &CommandBuilder::GetErrorMessage()
{
return mError;
}
OldStyleCommandPointer CommandBuilder::GetCommand()
{
wxASSERT(mValid);
@ -67,6 +62,14 @@ OldStyleCommandPointer CommandBuilder::GetCommand()
return result;
}
wxString CommandBuilder::GetResponse()
{
if (!mValid && !mError.empty()) {
return mError + wxT("\n");
}
return mResponse->GetResponse() + wxT("\n");
}
void CommandBuilder::Failure(const wxString &msg)
{
mError = msg;
@ -85,11 +88,11 @@ void CommandBuilder::BuildCommand(AudacityProject *project,
{
// Stage 1: create a Command object of the right type
auto scriptOutput = std::make_shared< ResponseQueueTarget >();
mResponse = std::make_shared< ResponseTarget >();
auto output
= std::make_unique<CommandOutputTargets>(std::make_unique<NullProgressTarget>(),
scriptOutput,
scriptOutput);
mResponse,
mResponse);
#ifdef OLD_BATCH_SYSTEM
OldStyleCommandType *factory = CommandDirectory::Get()->LookUp(cmdName);
@ -194,10 +197,7 @@ void CommandBuilder::BuildCommand(
cmdString.Trim(true); cmdString.Trim(false);
int splitAt = cmdString.Find(wxT(':'));
if (splitAt < 0 && cmdString.Find(wxT(' ')) >= 0) {
mError = wxT("Command is missing ':'");
ResponseQueueTarget::sResponseQueue().AddResponse(
Response{wxT("\n")});
mValid = false;
Failure(wxT("Syntax error!\nCommand is missing ':'"));
return;
}

View File

@ -19,6 +19,8 @@
#include "../MemoryX.h"
class AudacityProject;
class ResponseTarget;
using ResponseTargetPointer = std::shared_ptr<ResponseTarget>;
class OldStyleCommand;
using OldStyleCommandPointer = std::shared_ptr<OldStyleCommand>;
class wxString;
@ -30,6 +32,7 @@ class CommandBuilder
{
private:
bool mValid;
ResponseTargetPointer mResponse;
OldStyleCommandPointer mCommand;
wxString mError;
@ -45,6 +48,6 @@ class CommandBuilder
~CommandBuilder();
bool WasValid();
OldStyleCommandPointer GetCommand();
const wxString &GetErrorMessage();
wxString GetResponse();
};
#endif /* End of include guard: __COMMANDBUILDER__ */

View File

@ -463,9 +463,3 @@ void StatusBarTarget::Update(const wxString &message)
{
mStatus.SetStatusText(message, 0);
}
ResponseQueue &ResponseQueueTarget::sResponseQueue()
{
static ResponseQueue queue;
return queue;
}

View File

@ -57,7 +57,7 @@ and sends it to that message target.
#include "../MemoryX.h"
#include <vector>
#include "../commands/ResponseQueue.h"
#include <wx/thread.h>
class wxStatusBar;
@ -221,33 +221,35 @@ public:
void Update(const wxString &message) override;
};
/// Adds messages to the global response queue (to be sent back to a script)
class ResponseQueueTarget final : public CommandMessageTarget
/// Constructs a response (to be sent back to a script)
class ResponseTarget final : public CommandMessageTarget
{
private:
wxSemaphore mSemaphore;
wxString mBuffer;
public:
static ResponseQueue &sResponseQueue();
ResponseQueueTarget()
: mBuffer( wxEmptyString )
ResponseTarget()
: mBuffer(wxEmptyString),
mSemaphore(0, 1)
{
// Cater for handling long responses quickly.
mBuffer.Alloc(40000);
}
virtual ~ResponseQueueTarget()
virtual ~ResponseTarget()
{
if( mBuffer.StartsWith("\n" ) )
mBuffer = mBuffer.Mid( 1 );
sResponseQueue().AddResponse( mBuffer );
sResponseQueue().AddResponse(wxString(wxT("\n")));
}
void Update(const wxString &message) override
{
mBuffer += message;
#if 0
mResponseQueue.AddResponse(message);
#endif
}
virtual void Flush() override
{
mSemaphore.Post();
}
wxString GetResponse()
{
mSemaphore.Wait();
return mBuffer;
}
};

View File

@ -26,150 +26,80 @@ code out of ModuleManager.
#include "AppCommandEvent.h"
#include "../Project.h"
#include <wx/app.h>
#include <wx/window.h>
#include <wx/string.h>
#include <thread>
// Declare static class members
CommandHandler *ScriptCommandRelay::sCmdHandler;
tpRegScriptServerFunc ScriptCommandRelay::sScriptFn;
void ScriptCommandRelay::SetRegScriptServerFunc(tpRegScriptServerFunc scriptFn)
{
sScriptFn = scriptFn;
}
void ScriptCommandRelay::SetCommandHandler(CommandHandler &ch)
{
sCmdHandler = &ch;
}
/// Calls the script function, passing it the function for obeying commands
void ScriptCommandRelay::Run()
{
wxASSERT( sScriptFn != NULL );
while( true )
sScriptFn(&ExecCommand);
}
/// Send a command to a project, to be applied in that context.
void ScriptCommandRelay::PostCommand(
wxWindow *pWindow, const OldStyleCommandPointer &cmd)
{
wxASSERT( pWindow );
wxASSERT(cmd != NULL);
if ( pWindow ) {
AppCommandEvent ev;
ev.SetCommand(cmd);
pWindow->GetEventHandler()->AddPendingEvent(ev);
}
}
/// This is the function which actually obeys one command. Rather than applying
/// the command directly, an event containing a reference to the command is sent
/// to the main (GUI) thread. This is because having more than one thread access
/// the GUI at a time causes problems with wxwidgets.
int ExecCommand(wxString *pIn, wxString *pOut)
/// This is the function which actually obeys one command.
static int ExecCommand(wxString *pIn, wxString *pOut, bool fromMain)
{
{
CommandBuilder builder(::GetActiveProject(), *pIn);
if (builder.WasValid())
{
OldStyleCommandPointer cmd = builder.GetCommand();
ScriptCommandRelay::PostCommand(wxTheApp->GetTopWindow(), cmd);
*pOut = wxEmptyString;
}
else
{
*pOut = wxT("Syntax error!\n");
*pOut += builder.GetErrorMessage() + wxT("\n");
}
}
// Wait until all responses from the command have been received.
// The last response is signalled by an empty line.
wxString msg = ScriptCommandRelay::ReceiveResponse().GetMessage();
while (msg != wxT("\n"))
{
//wxLogDebug( "Msg: %s", msg );
*pOut += msg + wxT("\n");
msg = ScriptCommandRelay::ReceiveResponse().GetMessage();
}
return 0;
}
/// This is the function which actually obeys one command. Rather than applying
/// the command directly, an event containing a reference to the command is sent
/// to the main (GUI) thread. This is because having more than one thread access
/// the GUI at a time causes problems with wxwidgets.
int ExecCommand2(wxString *pIn, wxString *pOut)
{
{
CommandBuilder builder(::GetActiveProject(), *pIn);
if (builder.WasValid())
{
OldStyleCommandPointer cmd = builder.GetCommand();
AppCommandEvent ev;
ev.SetCommand(cmd);
// Use SafelyProcessEvent, which stops exceptions, because this is
// expected to be reached from within the XLisp runtime
wxTheApp->SafelyProcessEvent( ev );
*pOut = wxEmptyString;
if (fromMain)
{
// Use SafelyProcessEvent, which stops exceptions, because this is
// expected to be reached from within the XLisp runtime
wxTheApp->SafelyProcessEvent(ev);
}
else
{
// Send the event to the main thread
wxTheApp->AddPendingEvent(ev);
}
}
else
{
*pOut = wxT("Syntax error!\n");
*pOut += builder.GetErrorMessage() + wxT("\n");
}
}
// Wait until all responses from the command have been received.
// The last response is signalled by an empty line.
//
// LLL: Allow ExecCommand() to process the responses, otherwise
// it will hang waiting for more responses that will not be
// forthcoming.
#if 0
wxString msg = ScriptCommandRelay::ReceiveResponse().GetMessage();
while (msg != wxT("\n"))
{
//wxLogDebug( "Msg: %s", msg );
*pOut += msg + wxT("\n");
msg = ScriptCommandRelay::ReceiveResponse().GetMessage();
// Wait for and retrieve the response
*pOut = builder.GetResponse();
}
#endif
return 0;
}
/// Executes a command in the worker (script) thread
static int ExecFromWorker(wxString *pIn, wxString *pOut)
{
return ExecCommand(pIn, pOut, false);
}
/// Executes a command on the main (GUI) thread.
static int ExecFromMain(wxString *pIn, wxString *pOut)
{
return ExecCommand(pIn, pOut, true);
}
/// Starts the script server
void ScriptCommandRelay::StartScriptServer(tpRegScriptServerFunc scriptFn)
{
wxASSERT(scriptFn != NULL);
auto server = [](tpRegScriptServerFunc function)
{
while (true)
{
function(ExecFromWorker);
}
};
std::thread(server, scriptFn).detach();
}
// The void * return is actually a Lisp LVAL and will be cast to such as needed.
extern void * ExecForLisp( char * pIn );
extern void * nyq_make_opaque_string( int size, unsigned char *src );
extern void * nyq_reformat_aud_do_response(const wxString & Str);
void * ExecForLisp( char * pIn ){
wxString Str1( pIn );
wxString Str2;
ExecCommand2( &Str1, &Str2 );
// wxString provides a const char *
//const char * pStr = static_cast<const char*>(Str2);
// We'll be passing it as a non-const unsigned char *
// That 'unsafe' cast is actually safe. nyq_make_opaque_string is just copying the string.
void * pResult = nyq_reformat_aud_do_response( Str2 );
return pResult;
};
/// Gets a response from the queue (may block)
Response ScriptCommandRelay::ReceiveResponse()
void * ExecForLisp( char * pIn )
{
return ResponseQueueTarget::sResponseQueue().WaitAndGetResponse();
wxString Str1(pIn);
wxString Str2;
ExecFromMain(&Str1, &Str2);
return nyq_reformat_aud_do_response(Str2);
}

View File

@ -20,36 +20,15 @@
#include "../MemoryX.h"
class wxWindow;
class CommandHandler;
class Response;
class OldStyleCommand;
using OldStyleCommandPointer = std::shared_ptr<OldStyleCommand>;
class wxString;
typedef int (*tpExecScriptServerFunc)( wxString * pIn, wxString * pOut);
typedef int (*tpRegScriptServerFunc)(tpExecScriptServerFunc pFn);
extern "C" {
AUDACITY_DLL_API int ExecCommand(wxString *pIn, wxString *pOut);
} // End 'extern C'
typedef int(*tpExecScriptServerFunc)(wxString * pIn, wxString * pOut);
typedef int(*tpRegScriptServerFunc)(tpExecScriptServerFunc pFn);
class ScriptCommandRelay
{
private:
// N.B. Static class members also have to be declared in the .cpp file
static CommandHandler *sCmdHandler;
static tpRegScriptServerFunc sScriptFn;
public:
static void SetRegScriptServerFunc(tpRegScriptServerFunc scriptFn);
static void SetCommandHandler(CommandHandler &ch);
static void Run();
static void PostCommand(
wxWindow *pWindow, const OldStyleCommandPointer &cmd);
static Response ReceiveResponse();
public:
static void StartScriptServer(tpRegScriptServerFunc scriptFn);
};
#endif /* End of include guard: __SCRIPT_COMMAND_RELAY__ */

View File

@ -1449,9 +1449,7 @@ bool NyquistEffect::ProcessOne()
// communicated back to C++
auto msg = Verbatim( NyquistToWxString(nyx_get_string()) );
if (!msg.empty()) { // Empty string may be used as a No-Op return value.
if (!mExternal) {
Effect::MessageBox( msg );
}
Effect::MessageBox( msg );
}
else {
return true;