Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bin files not closed after reading #29

Closed
musantro opened this issue Mar 15, 2023 · 12 comments
Closed

bin files not closed after reading #29

musantro opened this issue Mar 15, 2023 · 12 comments

Comments

@musantro
Copy link

musantro commented Mar 15, 2023

Hi Kutu,

First of all thank you for this library!

I've been using *.bin files and I receive all the time that tracemalloc RuntimeWarning every time I ir.startup(test_file=my_bin_file).

Looking at the source code I see that the file is opened but not closed after reading. Is this on purpose?

pyirsdk/irsdk.py

Lines 415 to 418 in ad72f97

if test_file:
f = open(test_file, 'rb')
self._shared_mem = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
self.__is_using_test_file = True

Right now I am supressing that warning to not overwhelm my console log, but I was wondering if this could be fixed somehow.

I am happy to help by sending a PR fixing this if you want but maybe there's something that i am missing out.

Thanks again in advance!

@musantro musantro changed the title file not closed after reading bin files not closed after reading Mar 15, 2023
@kutu
Copy link
Owner

kutu commented Mar 15, 2023

i suppose

self._shared_mem.close()

should close it when you invoke shutdown()

@musantro
Copy link
Author

musantro commented Mar 15, 2023

Yes, I think too that actually it is closing, but I am afraid the warning comes from the local variable f, rather than self._shared_mem.

I was wondering if it is possible to do something like this:

if test_file:
    with open(test_file, 'rb') as f:
        self._shared_mem = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
        #....

@kutu
Copy link
Owner

kutu commented Mar 15, 2023

maybe file will be closed as soon as you exit with context
try copy library into your project local folder and do the changes you think might work

@musantro
Copy link
Author

OK, I will definitely try and bring up the results

@musantro
Copy link
Author

musantro commented Mar 15, 2023

so, using a file context:

if test_file:
    with open(test_file, 'rb') as f:
        self._shared_mem = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
        self.__is_using_test_file = True

and with this data:
image

and with this simple test:

import re
from pathlib import Path
from unittest import TestCase

import irsdk


class IrsdkTest(TestCase):
    client = irsdk.IRSDK()

    def setUp(self) -> None:
        # import warnings
        # warnings.filterwarnings(action="ignore", message="unclosed", category=ResourceWarning)
        ...

    def tearDown(self) -> None:
        self.client.shutdown()


class BasicsTest(IrsdkTest):

    def test_data_is_good(self):
        """Test if data is well logged"""
        car_idx = 16

        files = list(Path("tests/resources/mc_p217_mosport").glob("*.bin"))

        self.assertTrue(files)

        for file in files:
            with self.subTest(file=file.name):
                self.client.startup(test_file=str(file))

                self.assertEqual(
                    self.client['DriverInfo']['DriverCarIdx'], car_idx,
                    "Current player should be CarIdx = 16"
                )

                file_lap = re.match(r'^lap(\d{0,3})\.bin', file.name).group(1)

                self.assertEqual(
                    self.client['CarIdxLap'][car_idx], int(file_lap),
                    "Lap from data dump should match with file_name"
                )

                self.tearDown()

Test works and no RuntimeWarning is printed out:


Ran 1 test in 0.317s

OK

Process finished with exit code 0

Note that I am directly checking info from the bin file, DriverCarIdx and CarIdxLap. And I've also commented the part where I disable the RuntimeWarning.

If I reset the code in irsdk:

C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap80.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap81.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap82.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap83.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap84.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap85.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap86.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap87.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap88.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap89.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap90.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap91.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap92.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap93.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap94.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap95.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap96.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap97.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap98.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\...\tests\test_basics.py:32: ResourceWarning: unclosed file <_io.BufferedReader name='tests\\resources\\mc_p217_mosport\\lap99.bin'>
  self.client.startup(test_file=str(file))
ResourceWarning: Enable tracemalloc to get the object allocation traceback


Ran 1 test in 0.313s

OK

@kutu
Copy link
Owner

kutu commented Mar 15, 2023

you can open the file using the os.open() function, which returns a file descriptor directly (the file still needs to be closed when done).

when done

maybe we should store file descriptor and close it in shutdown

try store self._shared_mem_file and then close it in shutdown

@musantro
Copy link
Author

musantro commented Mar 15, 2023

Sorry Kutu, I forgot to copy and paste here the self.tearDown() which actually calls IRSDK.shutdown(). Edited above..

Also sorry about your last comment but I didn't understand; when done is when you put that resource into shared memory, and you store it into self._shared_mem. So when you create the property _shared_mem then you could close f without issues. Then afterwards, when you call shutdown() method as you said, the _shared_mem is closed.

Just to clarify, in my test, in every attempt to read a new bin file, i am using startup(test_file=bin_file), then access values, and shutdown(). With the context it is working and no RuntimeWarning.

@musantro
Copy link
Author

musantro commented Mar 15, 2023

I've made more tests using different python versions to ensure consistency.

The above change does not print RuntimeWarning and reading values still works for:

3.11.1 ✅
3.7.9 ✅
3.8.10 ✅

@kutu
Copy link
Owner

kutu commented Mar 15, 2023

the quote was from mmap docs, which says that you should close file descriptor only when you done

try with this version https://www.dropbox.com/s/rqmu1dygy1ainna/irsdk.py?dl=1

@musantro
Copy link
Author

Ok, I think you mean this url:
https://docs.python.org/3/library/mmap.html#module-mmap

In that paragraph:

If you wish to map an existing Python file object, use its fileno() method to obtain the correct value for the fileno parameter. Otherwise, you can open the file using the os.open() function, which returns a file descriptor directly (the file still needs to be closed when done).

I still understand that "when done" refers to "when you create the shared memory resource", but it seems pretty ambiguous to me...

I would go to use local variable and avoid to save that into a class attribute, but if you want to save into private attribute the _shared_mem_file and close it in IRSDK.shutdown(), its okay to me too :)

I've tested your code and still works for the 3 python versions.

kutu added a commit that referenced this issue Mar 15, 2023
@kutu
Copy link
Owner

kutu commented Mar 15, 2023

with that logic you can do

f = open(...)
fn = f.fileno()
f.close()
mmap.mmap(fn...)

don't think its fine

@kutu kutu closed this as completed Mar 15, 2023
@musantro
Copy link
Author

No, I never meant to say that, but rather after you call mmap.mmap()

f = open(...)
fn = f.fileno()
mmap.mmap(fn...)
f.close()
del f # what python does once startup() method finished

The snippet you passed never should work. Indeed:

    self._shared_mem = mmap.mmap(fn, 0, access=mmap.ACCESS_READ)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 9] Bad file descriptor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants