You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Neiss, Guenter" <GU...@plarad.de> on 2008/07/02 12:18:24 UTC
Bug: stdin can't be used on Windows, because of missing EOF
I tried all the things mentioned at: http://subversion.tigris.org/bugs.html
But didn't find that this bug/problem was already mentioned..
So I issue this report.
Just to be complete here is my environment:
OS: SVNClient: WinXP, SVNServe: Windows Server 2003
SVN: Release: 1.4.2 & 1.4.4 (but I am pretty shure that it's the same with 1.5, can't upgrade at the moment)
Proble description:
When running a hooks script, that get's some information via stdin (pre-revprop-change for ex.) this can't be used inside a simple BAT script.
IMHO this comes, because stdin lacks an EOF.
For Windows BAT file this may be implemented by simply writing EOF after writing the value into stdin, but this isn't the right way (IMHO).
The correct way is to create the (tmporary) file copy it's handle,
then use one handle to write & close the file
And use the other to pass it to the child process.
This handle was closed when the child process has terminated, which results in the file being deleted (because it's created with 'del_on_close').
When doing so the child process see a real EOL even if he reads the file in binary mode.
IMHO this is valid under Windows as well as under *nix (unix, linux)...
I locate the problem inside 'hooks.c' inside 'libsvn_repos'' (using a checkout from http://svn.collab.net/repos/svn/trunk/subversion/libsvn_repos Revision 31970).
The real work (creating the temporary file) is done inside function 'create_temp_file':
/* Create a temporary file F that will automatically be deleted when it is
closed. Fill it with VALUE, and leave it open and rewound, ready to be
read from. */
static svn_error_t *
create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t *pool)
{
const char *dir;
apr_off_t offset = 0;
SVN_ERR(svn_io_temp_dir(&dir, pool));
SVN_ERR(svn_io_open_unique_file2(f, NULL,
svn_path_join(dir, "hook-input", pool),
"", svn_io_file_del_on_close, pool));
SVN_ERR(svn_io_file_write_full(*f, value->data, value->len, NULL, pool));
SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
return SVN_NO_ERROR;
}
This should be changed to:
I look the first time at the subversion source code, so please forgive me if I don't understood all methods use therein ;-)
/* Create a temporary file F that will automatically be deleted when it is
closed. Fill it with VALUE, and leave it open and rewound, ready to be
read from. */
static svn_error_t *
create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t *pool)
{
const char *dir;
apr_off_t offset = 0;
apr_file_t *wf; // the handle we use to write to
SVN_ERR(svn_io_temp_dir(&dir, pool));
SVN_ERR(svn_io_open_unique_file2(&wf, NULL,
svn_path_join(dir, "hook-input", pool),
"", svn_io_file_del_on_close, pool));
// only principell, must be replaced by the functions used inside SVN
*f = duplicate_handle( wf, pool ); // create the handle pased back to the caller
SVN_ERR(svn_io_file_write_full(wf, value->data, value->len, NULL, pool));
// dont know how to enshure *f was closed on error here
// no need to seek on this handle
// SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
SVN_ERR(svn_io_file_close(wf, pool));
// dont know how to enshure *f was closed on error here
return SVN_NO_ERROR;
}
Hope that is enaught information :-)
MfG
G. Neiß
##########################################
# Herr Dipl.-Ing. Günter Neiß #
# Maschinenfabrik Wagner GmbH & Co.KG #
# Birrenbachshöhe 17 #
# D-53798 Much #
# Tel.: +49 2245 62-18 #
# Fax: +49 2245 62-55 #
# mailto:G.Neiss@plarad.de #
# http://www.plarad.de #
# #
# Handelsregister Siegburg HRA 1704 #
# Pers. Haftende Ges.: Wagner GmbH #
# Handelsregister Siegburg HRB 331 #
# Geschäftsführer: Paul-Heinz Wagner #
##########################################
Re: Bug: stdin can't be used on Windows, because of missing EOF
Posted by Martin Furter <mf...@rola.ch>.
On Wed, 2 Jul 2008, Neiss, Guenter wrote:
>
> /* Create a temporary file F that will automatically be deleted when it is
> closed. Fill it with VALUE, and leave it open and rewound, ready to be
> read from. */
> static svn_error_t *
> create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t *pool)
> {
> const char *dir;
> apr_off_t offset = 0;
>
> SVN_ERR(svn_io_temp_dir(&dir, pool));
> SVN_ERR(svn_io_open_unique_file2(f, NULL,
> svn_path_join(dir, "hook-input", pool),
> "", svn_io_file_del_on_close, pool));
> SVN_ERR(svn_io_file_write_full(*f, value->data, value->len, NULL, pool));
> SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
> return SVN_NO_ERROR;
> }
>
Looking at the code I see this function only used to create a file for use
as stdin of a hook script.
Wouldn't it be better to start the hook with a pipe as stdin and write the
contents of 'value' directly to the pipe instead of creating a temp file?
Just my 2 cents.
Martin
PS: Issue #2791 contains two patches which add support for pipes, i guess
it wouldn't be hard to add the stdin pipe for hooks.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Bug: stdin can't be used on Windows, because of missing EOF
Posted by "G.Nei�" <g....@plarad.de>.
> Sorry, I don't mean to be dense, but I read your report twice and didn't
> quite understand the problem.
> Can you describe the exact symptoms?
> Give an actual BAT script, explain what it's trying to do, and show us
> how it is prevented from doing that thing.
OK, first here is a simple script that exhibit the problem.
-----------------------------------------
rem choose a valid filepath for your system
set LogFile=C:\Temp\mylogfile.log
rem put something into the log, so we can see it's working
echo pre-revprop-change: start >%LogFile%
echo REPOSITORY: %~1 >>%LogFile%
rem Using copy or type will result in a 'hang' of the script
rem You will see (inside sysinternals taskmanager) a cmd.exe as a child of
svnserve
echo New Property Value (via find): >>%LogFile%
find /V "" >>%LogFile%
echo find done >>%LogFile%
echo New Property Value (via copy): >>%LogFile%
copy CON: >>%LogFile%
echo copy done >>%LogFile%
echo New Property Value (via type): >>%LogFile%
type CON: >>%LogFile%
echo type done >>%LogFile%
exit /B 0
-----------------------------------------
If You use this as the 'pre-revprop-change.bat', svnserve will be blocked..
If You examine the log file it generates you will find, that the script
'hangs' at the copy/type command.
IMHO it hangs, because the file passed as stdin to the script was never
closed, so windows doesn't set the file size.
This in turn leads to:
Any process that reads from this file will be blocked inside fgetc( stdin )
after he has read all bytes written so far.
This is, because reading via fgetc under windows will return EOF under two
conditions:
- if the file is opened in ASCII mode and the actual byte inside the file is
an EOF (^Z)
in this case fgetc will return EOF even if the actual file size is bigger
(this was used a long time to put 'hidden' data into text files ;-)
- a read past the actual file size will retun EOF independent of the read
mode
None of the above cases are fullfilled when a hook was called, so fgetc
block forever :-(
... and IMHO this is correct behaviour, because the process reading from the
handle doesn't know if the writer is willing to write more data to the file
that he must process althougth (a normal behaviour of a pipe).
Before writing my report I had searched via google and inside the web-mail
archives, but didn't find a solution for this problem.
In the meanwhile I download the complete mailing list (via gmane).
A search inside shows exactly ONE solution.
The solution is to use
find /V "" >>%LogFile%
instead of
copy CON: >>%LogFile%
I find and tried it just a few moments ago and it works.
It seams that find don't use fgetc to parse it's input, so it wasn't
blocked.
So this is a working workaround for me (but I gues other people will run
into the same problem when trying to use stdin inside a hook under windows).
"Karl Fogel" <kf...@red-bean.com> schrieb im Newsbeitrag
news:877ic4ar2b.fsf@red-bean.com...
"Neiss, Guenter" <GU...@plarad.de> writes:
> I tried all the things mentioned at:
> http://subversion.tigris.org/bugs.html
> But didn't find that this bug/problem was already mentioned..
>
> So I issue this report.
>
> Just to be complete here is my environment:
> OS: SVNClient: WinXP, SVNServe: Windows Server 2003
> SVN: Release: 1.4.2 & 1.4.4 (but I am pretty shure that it's the same with
> 1.5,
> can't upgrade at the moment)
>
> Proble description:
> When running a hooks script, that get's some information via stdin
> (pre-revprop-change for ex.) this can't be used inside a simple BAT
> script.
>
> IMHO this comes, because stdin lacks an EOF.
Sorry, I don't mean to be dense, but I read your report twice and didn't
quite understand the problem. Can you describe the exact symptoms?
Give an actual BAT script, explain what it's trying to do, and show us
how it is prevented from doing that thing.
Thank you,
-Karl
> For Windows BAT file this may be implemented by simply writing EOF after
> writing the value into stdin, but this isn't the right way (IMHO).
>
> The correct way is to create the (tmporary) file copy it's handle,
> then use one handle to write & close the file
> And use the other to pass it to the child process.
> This handle was closed when the child process has terminated, which
> results in
> the file being deleted (because it's created with 'del_on_close').
>
> When doing so the child process see a real EOL even if he reads the file
> in
> binary mode.
>
> IMHO this is valid under Windows as well as under *nix (unix, linux)...
>
> I locate the problem inside 'hooks.c' inside 'libsvn_repos'' (using a
> checkout
> from http://svn.collab.net/repos/svn/trunk/subversion/libsvn_repos
> Revision
> 31970).
>
> The real work (creating the temporary file) is done inside function
> 'create_temp_file':
>
> /* Create a temporary file F that will automatically be deleted when it is
> closed. Fill it with VALUE, and leave it open and rewound, ready to be
> read from. */
> static svn_error_t *
> create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t
> *pool)
> {
> const char *dir;
> apr_off_t offset = 0;
>
> SVN_ERR(svn_io_temp_dir(&dir, pool));
> SVN_ERR(svn_io_open_unique_file2(f, NULL,
> svn_path_join(dir, "hook-input", pool),
> "", svn_io_file_del_on_close, pool));
> SVN_ERR(svn_io_file_write_full(*f, value->data, value->len, NULL,
> pool));
> SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
> return SVN_NO_ERROR;
> }
>
> This should be changed to:
>
> I look the first time at the subversion source code, so please forgive me
> if I
> don't understood all methods use therein ;-)
>
> /* Create a temporary file F that will automatically be deleted when it is
> closed. Fill it with VALUE, and leave it open and rewound, ready to be
> read from. */
> static svn_error_t *
> create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t
> *pool)
> {
> const char *dir;
> apr_off_t offset = 0;
> apr_file_t *wf; // the handle we use to write to
>
> SVN_ERR(svn_io_temp_dir(&dir, pool));
> SVN_ERR(svn_io_open_unique_file2(&wf, NULL,
> svn_path_join(dir, "hook-input", pool),
> "", svn_io_file_del_on_close, pool));
>
> // only principell, must be replaced by the functions used inside SVN
> *f = duplicate_handle( wf, pool ); // create the handle pased back to
> the
> caller
>
> SVN_ERR(svn_io_file_write_full(wf, value->data, value->len, NULL,
> pool));
> // dont know how to enshure *f was closed on error here
>
> // no need to seek on this handle
> // SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
>
> SVN_ERR(svn_io_file_close(wf, pool));
> // dont know how to enshure *f was closed on error here
>
> return SVN_NO_ERROR;
> }
>
>
> Hope that is enaught information :-)
>
>
>
>
> MfG
> G. Nei�
>
> ##########################################
> # Herr Dipl.-Ing. G�nter Nei� #
> # Maschinenfabrik Wagner GmbH & Co.KG #
> # Birrenbachsh�he 17 #
> # D-53798 Much #
> # Tel.: +49 2245 62-18 #
> # Fax: +49 2245 62-55 #
> # mailto:G.Neiss@plarad.de #
> # http://www.plarad.de #
> # #
> # Handelsregister Siegburg HRA 1704 #
> # Pers. Haftende Ges.: Wagner GmbH #
> # Handelsregister Siegburg HRB 331 #
> # Gesch�ftsf�hrer: Paul-Heinz Wagner #
> ##########################################
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Bug: stdin can't be used on Windows, because of missing EOF
Posted by Karl Fogel <kf...@red-bean.com>.
"Neiss, Guenter" <GU...@plarad.de> writes:
> I tried all the things mentioned at: http://subversion.tigris.org/bugs.html
> But didn't find that this bug/problem was already mentioned..
>
> So I issue this report.
>
> Just to be complete here is my environment:
> OS: SVNClient: WinXP, SVNServe: Windows Server 2003
> SVN: Release: 1.4.2 & 1.4.4 (but I am pretty shure that it's the same with 1.5,
> can't upgrade at the moment)
>
> Proble description:
> When running a hooks script, that get's some information via stdin
> (pre-revprop-change for ex.) this can't be used inside a simple BAT script.
>
> IMHO this comes, because stdin lacks an EOF.
Sorry, I don't mean to be dense, but I read your report twice and didn't
quite understand the problem. Can you describe the exact symptoms?
Give an actual BAT script, explain what it's trying to do, and show us
how it is prevented from doing that thing.
Thank you,
-Karl
> For Windows BAT file this may be implemented by simply writing EOF after
> writing the value into stdin, but this isn't the right way (IMHO).
>
> The correct way is to create the (tmporary) file copy it's handle,
> then use one handle to write & close the file
> And use the other to pass it to the child process.
> This handle was closed when the child process has terminated, which results in
> the file being deleted (because it's created with 'del_on_close').
>
> When doing so the child process see a real EOL even if he reads the file in
> binary mode.
>
> IMHO this is valid under Windows as well as under *nix (unix, linux)...
>
> I locate the problem inside 'hooks.c' inside 'libsvn_repos'' (using a checkout
> from http://svn.collab.net/repos/svn/trunk/subversion/libsvn_repos Revision
> 31970).
>
> The real work (creating the temporary file) is done inside function
> 'create_temp_file':
>
> /* Create a temporary file F that will automatically be deleted when it is
> closed. Fill it with VALUE, and leave it open and rewound, ready to be
> read from. */
> static svn_error_t *
> create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t *pool)
> {
> const char *dir;
> apr_off_t offset = 0;
>
> SVN_ERR(svn_io_temp_dir(&dir, pool));
> SVN_ERR(svn_io_open_unique_file2(f, NULL,
> svn_path_join(dir, "hook-input", pool),
> "", svn_io_file_del_on_close, pool));
> SVN_ERR(svn_io_file_write_full(*f, value->data, value->len, NULL, pool));
> SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
> return SVN_NO_ERROR;
> }
>
> This should be changed to:
>
> I look the first time at the subversion source code, so please forgive me if I
> don't understood all methods use therein ;-)
>
> /* Create a temporary file F that will automatically be deleted when it is
> closed. Fill it with VALUE, and leave it open and rewound, ready to be
> read from. */
> static svn_error_t *
> create_temp_file(apr_file_t **f, const svn_string_t *value, apr_pool_t *pool)
> {
> const char *dir;
> apr_off_t offset = 0;
> apr_file_t *wf; // the handle we use to write to
>
> SVN_ERR(svn_io_temp_dir(&dir, pool));
> SVN_ERR(svn_io_open_unique_file2(&wf, NULL,
> svn_path_join(dir, "hook-input", pool),
> "", svn_io_file_del_on_close, pool));
>
> // only principell, must be replaced by the functions used inside SVN
> *f = duplicate_handle( wf, pool ); // create the handle pased back to the
> caller
>
> SVN_ERR(svn_io_file_write_full(wf, value->data, value->len, NULL, pool));
> // dont know how to enshure *f was closed on error here
>
> // no need to seek on this handle
> // SVN_ERR(svn_io_file_seek(*f, APR_SET, &offset, pool));
>
> SVN_ERR(svn_io_file_close(wf, pool));
> // dont know how to enshure *f was closed on error here
>
> return SVN_NO_ERROR;
> }
>
>
> Hope that is enaught information :-)
>
>
>
>
> MfG
> G. Neiß
>
> ##########################################
> # Herr Dipl.-Ing. Günter Neiß #
> # Maschinenfabrik Wagner GmbH & Co.KG #
> # Birrenbachshöhe 17 #
> # D-53798 Much #
> # Tel.: +49 2245 62-18 #
> # Fax: +49 2245 62-55 #
> # mailto:G.Neiss@plarad.de #
> # http://www.plarad.de #
> # #
> # Handelsregister Siegburg HRA 1704 #
> # Pers. Haftende Ges.: Wagner GmbH #
> # Handelsregister Siegburg HRB 331 #
> # Geschäftsführer: Paul-Heinz Wagner #
> ##########################################
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org