[Spice-devel] [PATCH spice-streaming-agent] Detect and handle exception creating capture engine

[Christophe de Dinechin]

> On 20 Feb 2018, at 18:05, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>>> 
>>> On 20 Feb 2018, at 17:49, Frediano Ziglio <fziglio at redhat.com> wrote:
>>> 
>>> Currently exception from a plugin are not handled when creating
>>> a capture engine.
>>> Capture the exception and try to use another plugin instead of
>>> bailing out.
>>> This was tested with an experimental GStreamer plugin.
>>> 
>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>> ---
>>> src/concrete-agent.cpp | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
>>> index 0720782..112ef93 100644
>>> --- a/src/concrete-agent.cpp
>>> +++ b/src/concrete-agent.cpp
>>> @@ -113,7 +113,14 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture(const
>>> std::set<SpiceVideoCodecT
>>>        // check client supports the codec
>>>        if (codecs.find(plugin.second->VideoCodecType()) == codecs.end())
>>>            continue;
>>> -        FrameCapture *capture = plugin.second->CreateCapture();
>>> +
>>> +        FrameCapture *capture;
>>> +        try {
>>> +            capture = plugin.second->CreateCapture();
>>> +        } catch (const std::runtime_error &err) {
>>> +            syslog(LOG_ERR, "Error getting capture engine: %s",
>>> err.what());
>>> +            continue;
>>> +        }
>> 
>> I noticed that you usually capture std::runtime_error. Why not
>> std::exception? You still get “what”, and that lets you catch bad_alloc too.
>> 
>>>        if (capture) {
>>>            return capture;
>>>        }
> 
> Lot of classes which inherit from std::exception are not normal program
> exceptions but hard cases (beside std::logic_error) which are hard to
> deal with so I prefer to limit to std::runtime_error.

I understand your rationale.

I the next iteration of my top-level cleanup, I plan to introduce a top-level catch for std::exception, where I think it makes sense (i.e we are exiting anyway). So what you are saying is that if there is a bad_allloc in CreateCapture, it’s unsafe to continue, we should throw and let the top-level catch clause deal with it, correct?

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel