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

Issue with setBuffer #34

Open
ykempf opened this issue May 31, 2018 · 1 comment
Open

Issue with setBuffer #34

ykempf opened this issue May 31, 2018 · 1 comment

Comments

@ykempf
Copy link
Contributor

ykempf commented May 31, 2018

When setBuffer() is not being called, outputBuffer is a NULL pointer as initialised in the constructor. https://github.com/fmihpc/vlsv/blob/master/vlsv_writer.cpp#L53 It seems that this situation leads to malloc probblems/OOM crashes in Vlasiator in some cases when it tries to empty the buffer before writing at https://github.com/fmihpc/vlsv/blob/master/vlsv_writer.cpp#L549.

When setBuffer() is called with an argument of 0, which should behave similarly as the default behaviour when the function is not called when looking at the rest of the code, what happens is that outputBuffer is set as a well-behaving zero-length char array https://github.com/fmihpc/vlsv/blob/master/vlsv_writer.cpp#L830. This does not lead to the crashes otherwise detected.

Now looking at the top of setBuffer(), if the user calls setBuffer() a second time and there is data left in the buffer, it will be silently deallocated and a new one will be allocated. Should this call emptyBuffer() first? Or should the user simply be prevented from calling setBuffer() multiple times, and then the delete is not needed any more?

Until this is solved we recommend to call setBuffer(0) or with the desired buffer size once and only once before any writing sequence.

@ykempf
Copy link
Contributor Author

ykempf commented May 31, 2018

Sorry I now saw that the default in Vlasiator is not 0. It actually crashes also with a buffer size manually set to 0. So the behaviour for zero buffer size is ill-defined and should be fixed, as it seemed to me that the assumed default in the code was to do the standard default writing if the buffer size is 0, which would also be a sensible expectation from the user side. Probably then the emptyBuffer() call at line 830 (and similar if there is) should be bypassed if the buffer size is set to 0.

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

1 participant