Re: [PATCH/RFC] fast-import doc: deadlock...

Re: [PATCH/RFC] fast-import doc: deadlock avoidance in bidirectional mode

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:33

Jonathan Nieder [off-list ref] writes:

How about this?

I find all the explanation above this line (elided) understandable and
sensible.

The expected use of --cat-blob-fd is to loop it back to the frontend, and
that is obvious to _us_, but there is nothing in the existing text (or the
text added by this patch) that suggests that fact to the first time
reader.  "Frontends must consume" may not "click" without that knowledge.

quoted hunk

 Documentation/git-fast-import.txt |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index ec6ef311..0ea649f4 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt

@@ -942,6 +942,12 @@ This command can be used anywhere in the stream that comments are
 accepted.  In particular, the `cat-blob` command can be used in the
 middle of a commit but not in the middle of a `data` command.
 
+While in some cases the 'cat-blob' result will fit in the pipe buffer,
+allowing fast-import to continue processing additional commands, this
+is not guaranteed.  Frontends must consume the cat-blob response
+completely before performing any writes to fast-import that might
+block.
+
 `ls`
 ~~~~
 Prints information about the object at a path to a file descriptor

@@ -975,7 +981,12 @@ Reading from a named tree::
 
 See `filemodify` above for a detailed description of `<path>`.
 
-Output uses the same format as `git ls-tree <tree> {litdd} <path>`:
+While in some cases the 'ls' response will fit in the pipe buffer,
+allowing fast-import to continue processing additional commands, this
+is not guaranteed.  Frontends must consume the ls response completely
+before performing any writes to fast-import that might block.
+
+The 'ls' response uses the same format as `git ls-tree <tree> {litdd} <path>`:
 
 ====
 	<mode> SP ('blob' | 'tree' | 'commit') SP <dataref> HT <path> LF

[PATCH/RFC v2] fast-import doc: cat-blob and ls responses need to be consumed quickly

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:53:33

If fast-import's command pipe and the frontend's cat-blob/ls response
pipe are both filled, there can be a deadlock.  Luckily all existing
frontends consume any pending cat-blob/ls responses completely before
writing the next command.

Document the requirements so future frontend authors and users can be
spared from the problem, too.  It is not always easy to catch that
kind of bug by testing.

To set the scene, add some words of explanation to help the novice
understand that "cat-blob" and "ls" output are meant for consumption
by the frontend.

Reported-by: Junio C Hamano <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Junio C Hamano wrote:

The expected use of --cat-blob-fd is to loop it back to the frontend, and
that is obvious to _us_, but there is nothing in the existing text (or the
text added by this patch) that suggests that fact to the first time
reader.  "Frontends must consume" may not "click" without that knowledge.

True.  Here's a rough attempt (again against "master").

 Documentation/git-fast-import.txt |   44 +++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index ec6ef311..7b220c28 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt

@@ -98,9 +98,10 @@ OPTIONS
 	options.
 
 --cat-blob-fd=<fd>::
-	Specify the file descriptor that will be written to
-	when the `cat-blob` command is encountered in the stream.
-	The default behaviour is to write to `stdout`.
+	Write responses to `cat-blob` and `ls` queries to the
+	file descriptor <fd> instead of `stdout`.  Allows `progress`
+	output intended for the end-user to be separated from other
+	output.
 
 --done::
 	Require a `done` command at the end of the stream.

@@ -942,6 +943,9 @@ This command can be used anywhere in the stream that comments are
 accepted.  In particular, the `cat-blob` command can be used in the
 middle of a commit but not in the middle of a `data` command.
 
+See ``Responses To Commands'' below for details about how to read
+this output safely.
+
 `ls`
 ~~~~
 Prints information about the object at a path to a file descriptor

@@ -991,6 +995,9 @@ instead report
 	missing SP <path> LF
 ====
 
+See ``Responses To Commands'' below for details about how to read
+this output safely.
+
 `feature`
 ~~~~~~~~~
 Require that fast-import supports the specified feature, or abort if

@@ -1079,6 +1086,35 @@ If the `--done` command line option or `feature done` command is
 in use, the `done` command is mandatory and marks the end of the
 stream.
 
+Responses To Commands
+---------------------
+New objects written by fast-import are not available immediately.
+Most fast-import commands have no visible effect until the next
+checkpoint (or completion).  The frontend can send commands to
+fill fast-import's input pipe without worrying about how quickly
+they will take effect, which improves performance by simplifying
+scheduling.
+
+For some frontends, though, it is useful to be able to read back
+data from the current repository as it is being updated (for
+example when the source material describes objects in terms of
+patches to be applied to previously imported objects).  This can
+be accomplished by connecting the frontend and fast-import via
+bidirectional pipes:
+
+====
+	mkfifo fast-import-output
+	frontend <fast-import-output |
+	git fast-import >fast-import-output
+====
+
+A frontend set up this way can use `progress`, `ls`, and `cat-blob`
+commands to read information from the import in progress.
+
+To avoid deadlock, such frontends must completely consume any
+pending output from `progress`, `ls`, and `cat-blob` before
+performing writes to fast-import that might block.
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a

-- 
1.7.10

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help