Re: PATCH: pgagent fix for 'Caught unhandled unknown exception; terminating' bug in SQL steps

On Tue, Mar 31, 2015 at 6:26 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Tue, Mar 31, 2015 at 6:20 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Tue, Mar 31, 2015 at 8:45 AM, Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> On Thu, Mar 26, 2015 at 6:19 PM, Thomas Krennwallner <
>>> tk+pgsql(at)postsubmeta(dot)net> wrote:
>>>
>>>> Hi!
>>>>
>>>> Running pgagent 3.4.0 on Debian jessie with an SQL jobstep crashes the
>>>> worker thread with the error message
>>>>
>>>> Caught unhandled unknown exception; terminating
>>>>
>>>> Both job and jobstep status then remain as 'r' (running) until the
>>>> pgagent process quits. After restarting pgagent, which cleans up the
>>>> job status and sets it to 'd' (aborted), the job containing the SQL
>>>> jobstep is free to run again, but the next run will end up in the same
>>>> deadlock situation.
>>>>
>>>> I have tracked down the problem to DBconn::GetLastError(), which gets
>>>> called after each SQL jobstep execution in Job::Execute(): there,
>>>> DBconn::GetLastError() throws an exception whenever the last error
>>>> message is empty.
>>>>
>>>> The attached patch fixes this problem by adding missing bounds checks
>>>> to DBconn::GetLastError().
>>>>
>>> Thanks for the patch.
>>> But - I was thinking about another fix for the same.
>>> I used the wxString::Trim(trimRight=true) function in order to remove
>>> the white-spaces from the right side.
>>>
>>>
>>> Dave,
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> *diff --git a/connection.cpp b/connection.cppindex 6103c00..ccbb5c7
>>> 100644--- a/connection.cpp+++ b/connection.cpp@@ -314,14 +314,7 @@ int
>>> DBconn::ExecuteVoid(const wxString &query) wxString
>>> DBconn::GetLastError() { // Return the last error message, minus any
>>> trailing line ends- if (lastError.substr(lastError.length() - 2, 2)
>>> == wxT("\r\n")) // DOS- return lastError.substr(0,
>>> lastError.length() - 2);- else if
>>> (lastError.substr(lastError.length() - 1, 1) == wxT("\n")) //
>>> Unix- return lastError.substr(0, lastError.length() -
>>> 1);- else if (lastError.substr(lastError.length() - 1, 1) ==
>>> wxT("\r")) // Mac- return lastError.substr(0,
>>> lastError.length() - 1);- else- return
>>> lastError;+ return lastError.Trim(true); }*
>>>
>>> Do you think - above diff make sense?
>>> If yes - I will commit the code.
>>>
>>
>>
>> That does seem like it would do it.
>>
> Thanks.
> I will commit the changes.
>
I have committed the changes.
Thanks Thomas Krennwallner for the report and for the patch too.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> <http://www.enterprisedb.com>
>
>
> *http://www.linkedin.com/in/asheshvashi*
> <http://www.linkedin.com/in/asheshvashi>
>
>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>