Alternative fix for #443 and partial fix for #447

[saspyrate]

I suggested to fix #443 by sending the data in chunks and this is how it was fixed. On second thoughts however, I am not sure whether this will prevent hanging in all situations (we probably don't fully understand in which situations exactly the SAS process stops reading from stdin). Hence, I think it's better to use a thread for writing. My suggestion is contained in the first commit of this pull request. Feel free to change it or leave it.

The second commit addresses one of the problems mentioned in #447. The problem can be exhibited by the code

sas.submit("data _null_; t = sleep(1000, 1); run;")

which currently uses all available CPU resources when run in STDIO mode on Linux.

My simple fix simply adds a sleep(0.01) if no data was read from the SAS process. Again, feel free to fix it differently.

I will prepare another pull request later to address the remaining part of #447.

[saspyrate]

_UPDATE: I removed the last commit (for reference it's still available in quick_fix_447). A proper fix for #447, that also addresses the problem with the STDIO method described in Canceling submitted statements, is demonstrated here: saspyrate/sas_stdio._

This should fix #447. I resolved it by looking for the presence of an abort cancel in the SAS log and then call _endsas(). Now submit() will no longer hang and consume all system resources after receiving an abort cancel.

Could you check whether this is an adequate way of handling it?

[tomweber-sas]

ok, finally getting back to this. Been too many different things going on, both with this (these) and other things I've had to be addressing. So, this seems to have 2 changes in this one PR. One is fine (the conditional sleep when trying to retrieve log/lst). I maybe would set it at .001, just since that still does the same thing but won't slow down anything that get's done quick.

The use of a thread/queue for stdin, like I used for both stdout/err in the windows client case (cuz things don't work right on windows), make sense as a different way to implement the fix I pushed before. I've been playing with that too, but I did it for both win and linux, because the previous problem happens for both, so the fix needs to be for both. The fix I pushed before worked the same either way. I've implemented the thread/queue like you have, but for both win/linux, and there is an issue on the Win side (big surprise). On windows python, having this for STDIN eliminates the ability to interrupt the process, in one case, and to terminate the process if even trying to interrupt in the other case. Other than that, it does behave ok and is just another way to implement the original fix. If it behaved correctly on Win, I wouldn't have an issue changing the implementation to this way instead of the other. Here's a mention of this problem in the doc, and I played around with it on win to see that it is goofy like documented (it either kills the session or never interrupts the process so you can't break out of a long running submission): see queue.get(), second paragraph - https://docs.python.org/3.9/library/queue.html?highlight=queue#queue-objects

No longer being able to break (ctl-C) out of a submit isn't an acceptable regression, just to implement this differently. So, unless there's a way to resolve that (I tried the various cases but maybe could spend some more time on it), I can't really trade this implementation for the other which doesn't regress the windows client path. If we can solve that, it would be ok.

Out of curiosity, is there anything this implementation solves the the original doesn't?

Thanks! Tom

[tomweber-sas]

Since this causes a regression w/ the client on windows, I'm going to close this out; I can't take the change as is. Also, for the wait() in the submit loop part of this, I added that to the code base and it's in the current production release; V4.3.0. So that made it in, just not this version of the other fix using the thread and queue's.

Thanks, Tom