0001629: Shell vs. read(2) errors on the script

ID	Project	Category	View Status	Date Submitted	Last Update

0001629	1003.1(2016/18)/Issue7+TC2	Shell and Utilities	public	2023-01-15 17:30	2024-06-11 09:07

Reporter	mirabilos	Assigned To
Priority	normal	Severity	Editorial	Type	Clarification Requested
Status	Closed	Resolution	Accepted As Marked

Name	mirabilos
Organization	mksh
User Reference
Section	unsure which applies
Page Number	(page or range of pages)
Line Number	(Line or range of lines)
Interp Status	---
Final Accepted Text	0001629:0006210


Summary	0001629: Shell vs. read(2) errors on the script
Description	As indicated in <Pine.BSM.4.64L.2301081426320.6999@herc.mirbsd.org> on the mailing list, both GNU bash <https://savannah.gnu.org/support/index.php?110763> and mksh <https://bugs.launchpad.net/mksh/+bug/2002044> got reports that the shell does not error out on read errors when loading either the first or any subsequent block of the script to execute. Chet says that treating them as EOF is historical behaviour. I don’t have a preference either way as I can see benefit in both; in contrast to Chet however I do think that the exit status mandated (if any) does matter and would prefer a high one, or even suggesting that the shell sends itself a suitable signal (PIPE, BUS and HUP, in that order, came to mind) so that the script could even have installed a trap handler to catch this condition beforehand (and clean up).
Desired Action	Decide whether… ① either ⓐ keep to existing behaviour; read errors on the script are treated as EOF, and the shell is still required to exit with the errorlevel of the last command executed (if any; a read error on the first block of a script would equal executing a null command and therefore exit zero), or ⓑ that read errors on script input require exiting indicating an error in some way, and ② if 1b, how shells are supposed to treat these errors; options are at least ⓒ some code within 1‥125, as with other errors, ⓓ 126 as if the script was not executable (which will require changing 126 as it’s IIRC currently tied to ENOEXEC), ⓔ signalling itself with a suitable signal, ⓕ exiting with 128+signalnumber of the signal, ⓖ any other, possibly high, status. I dislike 2c (which the bug submitter suggests as he interprets the spec this way currently) for possible confusion with utility exit statūs (grep, diff, cURL, unifdef, etc). I’m not sure about 2d but it sounds good. As mentioned above, I’d prefer 2e iff 1b is decided on (I’m similarly good with 1a, I just want a well-argumented decision either way). If 2e is not palatable, I’d rank 2f almost as high as 2d. 2g has the potential of conflicting with 2f for possibly unrelated signals.
Tags	applied_after_i8d3, issue8

chet_ramey 2023-01-20 21:18 reporter bugnote:0006120	As part of the discussion on savannah, I looked into different shells' behavior. The only shell that reflects a read error into $? is yash. The rest, including bash, treat read errors the same as EOF and exit with the last command's exit status.

chet_ramey 2023-02-02 15:51 reporter bugnote:0006142	I see that the minutes of the 30 January meeting begin to address this. The summary says that this is "partially handled by ... 0000367". I assume that means for errors during the `read' builtin. It's far from clear that an EIO (or ESTALE, or whatever) error while reading a script is meant to be one of the "an error other than ...," but if it is, it's best to state that explicitly instead of hiding behind the "including, but not limited to" language.

chet_ramey 2023-02-04 17:46 reporter bugnote:0006143	I sent this to austin-group-l: I looked at the etherpad. I'm not sure who tested bash-5.2.2 (Nick?), but you ended up running something that a vendor -- probably Ubuntu via Debian -- modified to add an error message. I can't tell what `/tmp/script' is, but running bash-5.2.15 on RHEL7 $ ./bash --version GNU bash, version 5.2.15(5)-release (x86_64-pc-linux-gnu) Copyright (C) 2022 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software; you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. against the following script: $ cat x10 echo a; echo b; echo after: $? exits with status 0 after a read error: $ strace -e trace=read -e inject=read:error=ESTALE:when=7 ./bash ./x10 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0@\316\0\0\0\0\0\0"..., 832) = 832 read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0P\16\0\0\0\0\0\0"..., 832) = 832 read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0`&\2\0\0\0\0\0"..., 832) = 832 read(3, "MemTotal: 1862792 kB\nMemF"..., 1024) = 1024 read(3, "echo a; echo b;\necho after: $?\n", 80) = 31 read(255, "echo a; echo b;\necho after: $?\n", 31) = 31 a b after: 0 read(255, 0x26d0290, 31) = -1 ESTALE (Stale file handle) (INJECTED) +++ exited with 0 +++ You have to make sure you inject the error when bash is reading input for the parser (the fifth read is checking whether or not the script is a binary file). You'd get the same results with when=6. You didn't check dash, but it returns 0 as well. I assume the other ash- derived BSD shells behave similarly. yash is still the only shell that returns an error in this case.

geoffclare 2023-02-09 16:59 manager bugnote:0006145	The following is a proposed resolution for this bug, but it is not existing practice in almost all shells (they treat a read error like end-of-file) and so we would like feedback from shell authors to indicate whether they are willing to make changes to follow these new requirements. The rationale for requesting this change in behavior is that executing the partial line after getting a read error is really not a good thing for any shell to do. Consider a shell script that contains the command: rm -f -- .tmp If the shell successfully reads "rm -f -- " and then gets an error when it tries to read ".tmp", it will execute "rm -f -- ", resulting in data loss. Please send feedback by March 9, 2023. Add a row (D2.1 p2330) to the table in 2.8.1 Consequences of Shell Errors: Read error when reading commands \| shall exit 4 \| shall exit 4 \| yes and add a new note after the table: 4. If a partial command was read before the read error occurred, that partial command shall not be executed. Add to the exit status section of the sh utility on P3155 after L107008: 1-126 A read error was detected while reading commands. Change P3155, L107009-107011 in the exit status section of the sh utility from: 1-125 A non-interactive shell detected an error other than command_file* not found or executable, including but not limited to syntax, redirection, or variable assignment errors. to: 1-125 A non-interactive shell detected an error other than command_file not found, command_file not executable, or a read error while reading commands; including but not limited to syntax, redirection, or variable assignment errors.

mirabilos 2023-02-18 21:51 reporter bugnote:0006158 Last edited: 2023-02-18 21:54	I’m okay with the proposal. [DEL:-To clarify, this is for nōn-interactive shells only, and interactive shells treat it as EOF.-:DEL] (I only saw the table now, it doesn’t render well in lynx.) This changes read errors, independent of whether the shell is interactive or not, to require a diagnostic and for the shell to exit with an error code. I think we can go around the “lawmaking” concern by looking at whether “all” shell maintainers agree to make the change, and if so, add it as requirement soon because it’ll be existing practice in all shells in a short while? Can we not standardise on a code here, if we’re changing things anyway? From the proposal, 126 looks the most palatable to me, so change 1–126 to just 126 (again, if everyone agrees). For the record, I’m very much willing to make this change to mksh.

chet_ramey 2023-02-20 21:14 reporter bugnote:0006160	How, if at all, does this affect the behavior of `.'? I assume that this interpretation intends that read errors during execution of `.' are treated as fatal special builtin errors similar to pathname not found. If the shell parses the contents of the file as a program, the shell should handle read errors as it would when reading a shell script and treat them as a fatal error for `.', with the consequences that implies. Is that the intent? Another question is exactly what constitutes a `partial command'. If you have a line that reads echo a; echo b; echo c and you get a read error after the second `;', this isn't exactly a `partial command'. It's a perfectly valid list. Or is this more intended to address partial lines? Getting EOF while parsing a compound command like `while' is already a syntax error, so this doesn't seem to make a difference there. It's mostly lists and pipelines.

mirabilos 2023-02-20 22:47 reporter bugnote:0006161	The ‘.’ behaviour definitely needs specifying and testing, yeah. I like your interpretation of not terminating the outer/calling script but allowing it to handle the error. As for “partial commands”… this part is indeed tricky. I’d like for the specification of this to move away from partial commands and also not to partial lines (as in the mail discussion with Chet, Bart and Robert). This would complicate the implementation, at least for cases where the shell can use blocked/buffered reads. Without consideration for lines, I’d implement it in mksh like this, if the read(2) fails, then I just terminate immediately with an error (instead of an EOF). Anything that happened prior to this was executed, anything that would be executed if it were an EOF wouldn’t. Hmm, so maybe partial command is right-ish, except not command but shell-consumable (let’s not add this word to the standard but I’m using it here to brainstorm) (a command is only consumable once terminated, e.g. by ; or newline), but… hmm… … anyway. I would just put it in the place where more input is read. I experimented with this right now, and I can just plug a yyerror() in there (i.e. treating it as a simulated syntax error) and it’ll DTRT (even for dot) although with errorlevel 1 not 126. So this _seems_ to DTRT…

kre 2023-02-21 05:58 reporter bugnote:0006163	Rather that trying to deal with what a "partial command" might mean, I'd rather simply say that if exiting, the shell shall not execute any commands not yet already executed, except for the exit trap (if one has already been defined). If the shell is not exiting (for which the only case I can currently imagine is "command . file" in a non-interactive shell, or any "." command in an interactive one) then the exit status should be non-zero (I'm not sure it matters much if we attempt to define 126 for that, or just anything). Note that even an interactive shell "shall exit" if it gets a read error on its standard input, nothing else makes any sense. So, apart from adding something about read errors in the file used for a '.' command, I'd change note 4 (4) being added in section 2.8.1 to say 4 If a read error occurs, and the shell is to exit, the shell shall execute no further commands, other than any specified in a previously defined EXIT trap action. And for the case of a '.' command failing due to a read error *5 If a read error occurs reading the file specified as the argument to the '.' command, the shell shall execute no commands from that file it has not already executed. The read error is then treated as a failure of the '.' command. Does that make sense to everyone? Is it sufficient?

McDutchie 2023-02-22 17:25 reporter bugnote:0006167 Last edited: 2023-02-22 17:32	Re 0001629:0006145: "[...] so we would like feedback from shell authors to indicate whether they are willing to make changes to follow these new requirements." Absolutely. I will fix this in ksh 93u+m regardless of what POSIX decides, because a failure to read from a script after execution has commenced is a fatal error that warrants throwing a panic. The possible execution of a partial command is one reason for this, but partially executing a script is not better. Either way, an inconsistent state results.

McDutchie 2023-02-22 17:39 reporter bugnote:0006168	Re 0001629:0006158: "Can we not standardise on a code here, if we’re changing things anyway? From the proposal, 126 looks the most palatable to me, so change 1–126 to just 126 (again, if everyone agrees)." Status 126 indicates failure to execute. I think that would be a bit misleading as the real problem here is a script read error after execution has successfully commenced. I think 128 is the best exit status for such panic or fatal error conditions. It is the highest non-signal exit status (signals are > 128), and is not currently specified for anything (regular errors are < 126), leaving it free to signify that something unusual happened that requires urgent attention. A precedent is git, which uses status 128 for fatal errors.

hvd 2023-03-05 11:20 reporter bugnote:0006192	> Status 126 indicates failure to execute. I think that would be a bit misleading as the real problem here is a script read error after execution has successfully commenced. I agree. If existing programs or scripts have any special handling of exit code 126, they would rightly assume that no action has yet taken place and the failed command can safely be retried in another way. For read errors in the middle of a script that already partially executed, that is inappropriate. But we are talking about scripts on unreliable media. They are going to fail unpredictably. They may result in read errors, they may result in bogus reads resulting in syntax errors, they may result in bogus reads resulting in valid scripts that do something other than what is intended. Because of that, scripts cannot meaningfully install handlers for read errors of the top-level script. And because of that, personally, I am not seeing a real benefit to requiring a specific exit code, I cannot see that being used in any meaningful way. Shells already do not agree on what exit code to use for syntax errors, for unsupported options, etc. Shells already do not even agree on the exit code for scripts not being found, despite that actually being defined by POSIX. Mandating a particular exit code for this that cannot be meaningfully used, but not for the other ones that can, I see no point to it.

hvd 2023-03-06 19:07 reporter bugnote:0006195 Last edited: 2023-03-06 19:13	On the possible special handling for 'command .': it makes sense that this would be treated the same as a syntax error in a file read by the command, but shells are not in agreement on what to do there and POSIX is less clear than it could be. echo \< >test command . ./test echo still here In a non-interactive shell, what is the expected behaviour? "Consequences of Shell Errors" says that a non-interactive shell shall exit on "Shell language syntax error". The description of the command builtin says "If the command_name is the same as the name of one of the special built-in utilities, the special properties in the enumerated list at the beginning of Special Built-In Utilities shall not occur." Exiting on a syntax error is not one of the special properties in that list, so by my reading, that rule remains in full effect even in 'command .' and the fact that many shells print "still here" contradicts what I believe POSIX requires (without comment on whether POSIX is right to require this). That same reading also applies to the proposed wording in 0001629:0006145; a read error encountered during 'command .' should cause the shell to exit. This is a change I would be willing to make. I would not be happy to have these two cases be treated differently without a good reason, but if this is not the intended interpretation, I would also be happy to implement to-be-written alternative wording that would require the shell to survive syntax errors and read errors alike in 'command .'. However, in interactive shells, read errors in a '.' should definitely not require the shell to exit, only read errors in the top-level script should have that effect. If a user does e.g. '. /proc/self/mem' (reading from the start of /proc/self/mem will fail with EIO), there is no reason not to allow the user to read more commands, that fd is still perfectly fine. The suggested wording does require the shell to exit in that case.

geoffclare 2023-03-09 17:29 manager bugnote:0006196	New proposed resolution... Add a row (D2.1 p2330) to the table in 2.8.1 Consequences of Shell Errors: Read error when reading commands \| shall exit 4 \| shall exit 4 \| yes and add a new note after the table: 4. If a read error occurs when reading commands, other than from the file operand of the dot special built-in, the shell shall execute no further commands (including any already successfully read but not yet executed) other than any specified in a previously defined EXIT trap action. A read error while reading from the file operand of the dot special built-in shall be treated as a special built-in utility error. Add to the exit status section of the sh utility on P3155 after L107008: 128 A read error was detected while reading commands, except from the file operand of the dot special built-in. Change P3155, L107009-107011 in the exit status section of the sh utility from: 1-125 A non-interactive shell detected an error other than command_file not found or executable, including but not limited to syntax, redirection, or variable assignment errors. to: 1-125 A non-interactive shell detected an error other than command_file not found, command_file not executable, or a read error while reading commands (except from the file operand of the dot special built-in); including but not limited to syntax, redirection, or variable assignment errors.

hvd 2023-03-09 17:44 reporter bugnote:0006197	The new proposed resolution would require 'command . /some/file' to exit the shell on syntax errors, but would also require to not exit the shell on read errors. This is inappropriate. The new proposed resolution requires an exit status of 126. This is also inappropriate.

geoffclare 2023-03-10 09:20 manager bugnote:0006198	> The new proposed resolution would require 'command . /some/file' to exit the shell on syntax errors, but would also require to not exit the shell on read errors. True, and this is a defect for the syntax error case, as existing shells do not exit. > The new proposed resolution requires an exit status of 126. It requires 128, not 126. The consensus in the teleconference was to agree with the comments that said 126 would not be appropriate, and also with the comments that said 128 would be a good choice.

hvd 2023-03-10 10:57 reporter bugnote:0006199	> True, and this is a defect for the syntax error case, as existing shells do not exit. Some existing shells do not exit. Others do. > It requires 128, not 126. Apologies, I noticed the change to require a specific exit status and not that it had been changed from 126.

kre 2023-03-10 14:52 reporter bugnote:0006200	Re 0001629:0006197 0001629:0006198 0001629:0006199 : I agree it is probably a problem that shells exit on syntax errors after "command ." (also "command eval") but it is a problem in the sense that it would be better if that isn't what happened - and it isn't just "some shells" that exit, it is most of them. I also agree it would be better for syntax errors and read errors to be treated the same for this. That is where we get a problem - read error requirements are new, and we can more or less do what we like, since everyone has more or less agreed that "treat it like EOF" which just about everyone did is not the right thing to do, and shells are going to have to change anyway. But for syntax errors, most shells are doing what shells always have, and what the standard requires - there even might be applications depending upon this behaviour (though I don't think I have seen many "command eval" and even less, if any "command ." in real scripts - and bash and zsh don't bother exiting (bash in non-posix mode anyway) even without "command" so probably there aren't applications that will care, but we cannot know. In any case, this isn't a problem with the proposed resolution of this issue (read error behaviour) - a new bug needs to be filed and processed to get the behaviour of "command ." and "command eval" altered to be "not exit" (and for this, it may need to start out just being "may exit" rather than "shall not exit" with a future directions indicating that the stronger form will apply in a later version. Incidentally, it appears there is no way to prevent a shell from exiting on a syntax error in a trap command string (action) - those are run "in a matter equivalent to eval action" which must include in the interpretation, as if by a special builtin, so if there is a failure to parse the action, the shell is currently required to exit (non-interactive shells) which is probably sub-optimal. (I haven't yet tested to see what shells actually do for this).

chet_ramey 2023-03-10 21:33 reporter bugnote:0006201	I'm going to change the bash behavior to exit immediately with status 128 on read errors while reading commands from a script. I'm going to hold off changing any other behavior. Bash already reads an entire `.' file before parsing and executing any commands in it, so that's not going to be a problem. I'd like to see the behavior of `command .' and `command eval' standardized so shells don't exit if the builtin fails, even with a read error, but I'm willing to have the discussion.

geoffclare 2023-03-20 16:21 manager bugnote:0006210	Add a row (D2.1 p2330) to the table in 2.8.1 Consequences of Shell Errors: Unrecoverable read error when reading commands \| shall exit 4 \| shall exit 4 \| yes and add a new note after the table: 4. If an unrecoverable read error occurs when reading commands, other than from the file operand of the dot special built-in, the shell shall execute no further commands (including any already successfully read but not yet executed) other than any specified in a previously defined EXIT trap action. An unrecoverable read error while reading from the file operand of the dot special built-in shall be treated as a special built-in utility error. Change P3155, L107009-107011 in the exit status section of the sh utility from: 1-125 A non-interactive shell detected an error other than command_file not found or executable, including but not limited to syntax, redirection, or variable assignment errors. to: 1-125 A non-interactive shell detected an error other than command_file not found, command_file not executable, or an unrecoverable read error while reading commands (except from the file operand of the dot special built-in); including but not limited to syntax, redirection, or variable assignment errors. Add to the exit status section of the sh utility on P3155 after L107014: 128 An unrecoverable read error was detected while reading commands, except from the file operand of the dot special built-in. On D2.1 page 358 line 12462 section <stdlib.h> (RATIONALE), change: Exit statuses of 126, 127, and greater than 128 are ambiguous in certain circumstances because they have special meanings in the shell (see [xref to XCU 2.8.2]). to: Exit statuses of 126 and greater are ambiguous in certain circumstances because they have special meanings in the shell (see [xref to XCU 2.8.2] and the EXIT STATUS section of [xref to XCU sh]). On D2.1 page 359 line 12469 section <stdlib.h> (RATIONALE), delete: The value 128 is disallowed for simplicity, making the allowed values 1 to 125 inclusive rather than 1 to 125 inclusive and 128. After D2.1 page 531 line 18867 section _Exit() (APPLICATION USAGE), add a new paragraph: Exit statuses of 126 and greater are ambiguous in certain circumstances because they have special meanings in the shell (see [xref to XCU 2.8.2] and the EXIT STATUS section of [xref to XCU sh]). After D2.1 page 789 line 27009 section exit() (APPLICATION USAGE), add a new paragraph (after applying bug 1490): See also _Exit(). On D2.1 page 2370 line 76769 section exit (RATIONALE), change: As explained in other sections, certain exit status values have been reserved for special uses and should be used by applications only for those purposes: 126 A file to be executed was found, but it was not an executable utility. 127 A utility to be executed was not found. >128 A command was interrupted by a signal. to: As explained in other sections, certain exit status values have been reserved for special uses and should be used by applications only for those purposes: 126 A file to be executed was found, but it was not an executable utility. 127 A utility to be executed was not found. 128 An unrecoverable read error was detected by the shell while reading commands, except from the file operand of the dot special built-in. >128 A command was interrupted by a signal. On page 3238 line 110033 section tsort (RATIONALE), after applying bug 1617 change: Implementations are urged to set the implementation-defined maximum number of cycles reported via the exit status to at most 125, leaving 128 for other errors, and leaving 126, 127, and values greater than 128 to have the special meanings that the shell assigns to them. (An implementation that wants to distinguish other types of errors would need to set the maximum to less than 125 so that 128 is not the only code available for those errors). to: Implementations are urged to set the implementation-defined maximum number of cycles reported via the exit status to at most 124, leaving values above that maximum through 125 for other errors, and leaving values 126 and greater to have the special meanings that the shell assigns to them.

geoffclare 2023-06-13 11:08 manager bugnote:0006329	When applying this bug I noticed that the tsort FUTURE DIRECTIONS section needed to be changed in line with the change to the RATIONALE section, and I have made that change.

Date Modified	Username	Field	Change
2023-01-15 17:30	mirabilos	New Issue
2023-01-15 17:30	mirabilos	Name	=> mirabilos
2023-01-15 17:30	mirabilos	Organization	=> mksh
2023-01-15 17:30	mirabilos	URL	=> unsure which applies
2023-01-15 17:30	mirabilos	Section	=> unsure which applies
2023-01-20 21:18	chet_ramey	Note Added: 0006120
2023-01-30 16:45	nick	Relationship added	related to 0000367
2023-02-02 15:51	chet_ramey	Note Added: 0006142
2023-02-04 17:46	chet_ramey	Note Added: 0006143
2023-02-09 16:59	geoffclare	Note Added: 0006145
2023-02-09 17:06	geoffclare	Project	Online Pubs => 1003.1(2016/18)/Issue7+TC2
2023-02-18 21:51	mirabilos	Note Added: 0006158
2023-02-18 21:54	mirabilos	Note Edited: 0006158
2023-02-20 21:14	chet_ramey	Note Added: 0006160
2023-02-20 22:47	mirabilos	Note Added: 0006161
2023-02-21 05:58	kre	Note Added: 0006163
2023-02-22 17:25	McDutchie	Note Added: 0006167
2023-02-22 17:32	McDutchie	Note Edited: 0006167
2023-02-22 17:39	McDutchie	Note Added: 0006168
2023-03-05 11:20	hvd	Note Added: 0006192
2023-03-06 19:07	hvd	Note Added: 0006195
2023-03-06 19:10	hvd	Note Edited: 0006195
2023-03-06 19:11	hvd	Note Edited: 0006195
2023-03-06 19:13	hvd	Note Edited: 0006195
2023-03-09 17:29	geoffclare	Note Added: 0006196
2023-03-09 17:44	hvd	Note Added: 0006197
2023-03-10 09:20	geoffclare	Note Added: 0006198
2023-03-10 10:57	hvd	Note Added: 0006199
2023-03-10 14:52	kre	Note Added: 0006200
2023-03-10 21:33	chet_ramey	Note Added: 0006201
2023-03-20 15:48	eblake	Relationship added	related to 0001490
2023-03-20 16:21	geoffclare	Note Added: 0006210
2023-03-20 16:23	geoffclare	Page Number	=> (page or range of pages)
2023-03-20 16:23	geoffclare	Line Number	=> (Line or range of lines)
2023-03-20 16:23	geoffclare	Interp Status	=> ---
2023-03-20 16:23	geoffclare	Final Accepted Text	=> 0001629:0006210
2023-03-20 16:23	geoffclare	Status	New => Resolved
2023-03-20 16:23	geoffclare	Resolution	Open => Accepted As Marked
2023-03-20 16:24	geoffclare	Tag Attached: issue8
2023-03-20 16:28	eblake	Relationship added	related to 0001617
2023-06-13 11:08	geoffclare	Note Added: 0006329
2023-06-13 11:08	geoffclare	Status	Resolved => Applied
2023-06-13 11:09	geoffclare	Tag Attached: applied_after_i8d3
2024-06-11 09:07	agadmin	Status	Applied => Closed

View Issue Details

Relationships

Activities

Issue History

related to	0000367	Closed	ajosey	1003.1(2008)/Issue 7	interaction between readonly, export, getopts, and read
related to	0001490	Closed		1003.1(2016/18)/Issue7+TC2	warn app writers about flush errors not being detectable by exit()
related to	0001617	Closed		Issue 8 drafts	what tsort should do with cycles?