Re: PgAgent 3.3.0 batch scripts on windows always get status failed

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Bastiaan Olij <bastiaan(at)basenlily(dot)me>
Cc: pgAdmin Support <pgadmin-support(at)postgresql(dot)org>
Subject: Re: PgAgent 3.3.0 batch scripts on windows always get status failed
Date: 2013-02-04 15:49:00
Message-ID: CA+OCxozUYBetmUob8jmp4fDMcPkox-OgF9mfQnXfzb8p3HO=Nw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-support

Hi

On Thu, Jan 31, 2013 at 2:03 AM, Bastiaan Olij <bastiaan(at)basenlily(dot)me> wrote:
> Hi All,
>
> First post on this list so I hope I'm doing things right.
> We've had a problem for awhile now that any batch script run by pgagent
> on windows gets the status failed even if the batch file runs just fine.
>
> I downloaded the latest copy of the source code to have a look and found
> something strange in the code for Job::Execute in job.cpp
>
> At the start of this method the variable succeeded gets set to false, in
> the windows code it never gets set to true as apposed to the *nix code
> where the variable rc is loaded with the return status of the batch
> script and succeeded is assigned true if this status equals 0.
>
> In the windows code (line 268) rc is set to 1, the return status of the
> batch script is not returned. I do not know if this is done on purpose
> and that as we can't detect if the script was successful succeeded is
> left as failed or if not setting succeeded to true has simply been
> overlooked?
>
> I would suggest changing this code to:
> 268: // assume successful execution of script
> 269: rc = 0;
> 270: succeeded = true;
>
> Alternatively, I'm assuming the code for CloseHandle is calling pclose,
> the return value of pclose is the errorlevel returned by the batch
> script and could be assigned to rc. I couldn't find the code for
> CloseHandle so I'm not sure if it would be able to return this code but
> if so the code could be turned into:
> 267: rc = CloseHandle(h_script);
> 268: if (rc == 0)
> 269: succeeded = true;

This seems to be a previously reported issue that slipped through the
net due to lack of feedback. The patch I proposed looks like:

raptor:pgagent dpage$ git diff
diff --git a/job.cpp b/job.cpp
index a9c0c1b..937bc9c 100644
--- a/job.cpp
+++ b/job.cpp
@@ -264,8 +264,8 @@ int Job::Execute()
}

+ GetExitCodeProcess(h_script, (LPDWORD)&rc);
CloseHandle(h_script);
- rc = 1;

#else
// The *nix way.

Are you in a position to test it if I build a binary for Windows?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-support by date

  From Date Subject
Next Message Dan Halbert 2013-02-04 15:54:46 Re: 1.16.1: many operations cause display tree to collapse
Previous Message Dave Page 2013-02-04 15:37:32 Re: 1.16.1: many operations cause display tree to collapse