sassoftware / saspy

A Python interface module to the SAS System. It works with Linux, Windows, and Mainframe SAS as well as with SAS in Viya.
https://sassoftware.github.io/saspy
Other
367 stars 149 forks source link

ResourceWarning when used in unittest #543

Closed phchri closed 1 year ago

phchri commented 1 year ago

Describe the bug When using saspy submit in a unit test I get a subprocess.py ResourceWarning because a process is still running when it's del function is called. The following code reproduces this behavior on Python 3.6 reliably for me.

import saspy
import unittest

class BugUnitTest(unittest.TestCase):

    def test_ressource(self):
        with saspy.SASsession() as sas:
            sas.submit(f"%let foobar = 4; data foo;put\"foo\";run;")
            sas.symget("foobar")

if __name__ == "__main__":
    unittest.main()

To Reproduce Steps to reproduce the behavior:

  1. Submit the code above.
  2. Note a message like this in the output: ''' .../anaconda3/lib/python3.6/subprocess.py:786: ResourceWarning: subprocess 24234 is still running ResourceWarning, source=self) '''

Expected behavior The warning might not be a serious issue by disturbs unittesting output.

Configuration information Using a local SAS 9.40m8 installation on Linux. Python 3.6.8 :: Anaconda custom (64-bit)

Additional context I think I also know the fix for this. The subprocess that does not get cleaned up is started in line 217 of sasiostdio.py. From the Popen documentation it sounds like we need to add a x.wait() after line 226 (x.terminate()) to wait for the termination. Probably it's better to use it with a timeout, like x.wait(1).

tomweber-sas commented 1 year ago

Hey, I'm looking into this. I haven't been able to reproduce it yet. I've copied that code to a file (test.py) and have run it both as a batch script and with unittest, but didn't happen to see the warning. It might be a timing issue or even that I'm using python 11; something could have changed. But, I also am using a remote SAS session as my setinit is expired on the local windows SAS I have installed - have to fix that then I can try it locally to see if that matters. I'll keep trying some things and see if I see it though.

C:\saspy\saspy>python tests\test.py
SAS Connection established. Subprocess id is 18636

SAS Connection terminated. Subprocess id was 18636
.
----------------------------------------------------------------------
Ran 1 test in 3.832s

OK

C:\saspy\saspy>python -m unittest tests\test.py
SAS Connection established. Subprocess id is 20840

SAS Connection terminated. Subprocess id was 20840
.
----------------------------------------------------------------------
Ran 1 test in 3.889s

OK

C:\saspy\saspy>
tomweber-sas commented 1 year ago

I got my local SAS running again and got the same results with that. So, I'm not reproducing this with what I'm running. Did you try out your fix using wait(), and see it get rid of the message for your case - without causing any issues?

tomweber-sas commented 1 year ago

I don't know what I thought I saw that made me think you were running on windows. I've been way off base trying to run this there. I see you're on linux, using stdio. So, let me go try that and see what I see. Sorry bout that!

tomweber-sas commented 1 year ago

well, I got the same results on linux also. I did use 9.4m8, and my python on linux is 3.9.12.

tom64-7> python3 tests/test.py
SAS Connection established. Subprocess id is 2166013

SAS Connection terminated. Subprocess id was 2166013
.
----------------------------------------------------------------------
Ran 1 test in 4.553s

OK
tom64-7> python3 -m unittest tests/test.py
SAS Connection established. Subprocess id is 2166078

SAS Connection terminated. Subprocess id was 2166078
.
----------------------------------------------------------------------
Ran 1 test in 3.072s

OK
tom64-7> python3
Python 3.9.12 (main, Apr  5 2022, 06:56:58)
[GCC 7.5.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>
tomweber-sas commented 1 year ago

I don't see what version of saspy you're using. I am using the current code, 5.1.2. I made a change in that for the last issue and it is possible that change affects this behavior (for the better, if so). What version of SASPy are you running? Does this go away for you with 5.1.2?

phchri commented 1 year ago

Hi Tom, I used 5.1.2, got it from Github this morning, maybe it's related to the python version, specifically, unittest, I did not get this warning if I run outside of unittest.

tomweber-sas commented 1 year ago

Hmmm. Did you add the wait() and see if it made a difference for the case that gets the warning?

phchri commented 1 year ago

Yes, I made the fix and the resource warning disappeared. I just added wait(1) after terminate().

tomweber-sas commented 1 year ago

Well, I've played around with that and I don't see any problem adding that. Doesn't make a difference in any of the cases I'm running. Do you want to open a PR for that change? I can add it easily too, but if you want to contribute it since you found it, that's cool.

phchri commented 1 year ago

OK, I'll place a pull request next week. I'll also see if I can pinpoint what part of my setup makes this happen, whether it's the Python version or my sascfg_personal.py.

tomweber-sas commented 1 year ago

ok, sounds good!

phchri commented 1 year ago

I could not find out if this is related to the Python version I'm using but from all I could say adding the wait(1) is the right thing to do and running with it the last couple days I did not see any issues with it. Since I'm swamped with unrelated work, I would appreciate if you, @tomweber-sas , could just add the fix at your convenience.

tomweber-sas commented 1 year ago

Sure , no trouble, I can add that easily. I'll take care of it! Thanks, Tom