Re: [lmi] wxProgressDialog resists interruption with Cancel

[Vadim Zeitlin]

On Wed, 13 Nov 2013 20:35:00 +0000 Greg Chicares <address@hidden> wrote:

GC> With lmi HEAD revision 5827, a progress dialog cannot be interrupted
...
GC> What's so special about this command? My guess is that it uses
GC> wxExecute() to run a command line like this:
GC>   CMD /c C:/path/to/fop -fo file1 -pdf file2
GC> and wx doesn't detect the Cancel event due to the external process.

 Yes, this looks like the most likely explanation. However isn't it the
right thing to do in this case? After all, even if "Cancel" presses were
taken into account visually, this still wouldn't affect the running child
command, so the program still wouldn't be able to continue. I.e. while the
situation is clearly not ideal from the UI point of view, at least it is
consistent: "Cancel" doesn't work because this operation can't be
cancelled.

GC> Inserting this code from 'wx/src/generic/progdlgg.cpp':
GC>   wxEventLoopBase::GetActive()->YieldFor(wxEVT_CATEGORY_UI);
GC> into the main loop that uses the progress dialog didn't help much.

 It wouldn't, because this is the loop that already calls
wxProgressDialog::Update() which does exactly this internally, as you've
probably seen.

GC> Inserting it into the code that calls wxExecute() (patch below[0])
GC> is much more effective:

 I don't see how could this be possible. Maybe it seems effective just
because the child process terminates quickly in this case anyhow? If it
really printed 5000 pages instead of 5, this wouldn't be the case, of
course.

GC> I can break out of the loop with a single press of the Cancel key, as
GC> long as I press it when the busy cursor is not displayed. Questions:
GC> 
GC> - Is there any hidden danger in this patch? (I suspect not, based
GC> on our earlier discussion.)

 Not danger per se, but I think this patch is rather pointless.

GC> - Is there a better way to do this? (A commented-out line in the
GC> patch shows an unsuccessful SafeYieldFor() idea that I tried.)

 Just to explain the parenthesized remark: wxApp::SafeYieldFor(NULL)
doesn't work because it disables the progress dialog, to avoid this you'd
have to pass the pointer to the dialog to it instead of NULL. However in
this case there is not any difference between wxApp::SafeYieldFor() and
directly calling wxEventLoop::YieldFor() anyhow because the only safety the
former function adds is the disabling of all the windows (other than the
one specified as its argument) and in this case all the windows had been
already disabled by wxProgressDialog.

 But the question about how to do it better is more interesting, of course.
Unfortunately the answer to it is that it requires more work, i.e. it can't
be achieved by any calls to wxYield() and related functions but you need to
change the program to use asynchronous execution. I.e. you can't use
wxExecute(wxString, wxArrayString, wxArrayString) overload you use
currently which runs the command synchronously and returns the output and
will need to use wxExecute(wxString, wxEXEC_ASYNC, wxProcess*) instead and
collect the program output yourself from the streams obtained from the
associated wxProcess. Only this will allow you to really cancel the child
execution (either by killing it when "Cancel" is pressed or maybe just
letting it run to its peaceful end but ignoring its output) and return to
the normal processing in the main event loop.

 IMO this would be the right thing to do here, synchronous wxExecute()
should only be used for (very) short-running child processes and this is
clearly not the case here. But this does require totally changing the logic
of this code -- there is no way around it.

 Sorry if my answer doesn't really help you...
VZ