-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
LWJGLBufferAllocator does not initialize new buffers to zeros, ReflectionAllocator does #2176
Comments
This seems worth documenting, at least. |
Nice find. |
memCalloc is what I'd recommend... but only because the old grizzled C programmers with the horror that lurks deep in their eyes all switched to calloc decades ago... |
… of nmemAlloc() jMonkeyEngine#2176 LWJGLBufferAllocator.allocate() now always returns a zero-initialized buffer. This behavior is consistent with how ReflectionAllocator works, and Java in general.
… of nmemAlloc() jMonkeyEngine#2176 LWJGLBufferAllocator.allocate() now always returns zero-initialized buffers.
* #2176 Make LWJGLBufferAllocator use nmemCalloc() instead of nmemAlloc() #2176 LWJGLBufferAllocator.allocate() now always returns zero-initialized buffers. * Added unit tests for JmeExporter/JmeImporter implementations Tests all write* and read* methods of OutputCapsule and InputCapsule respectively. * Fixed XMLExporter/XMLImporter BitSets Previously DOMOutputCapsule was writing indices of set bits and DOMInputCapsule was reading values of each bit. Changed DOMOutputCapsule to match the expected behavior of DOMInputCapsule. * Fixed DOMInputCapsule.readString() returning defVal for empty strings org.w3c.dom.Element.getAttribute() returns an empty string for attributes that are not found. It looks like DOMInputCapsule.readString() was interpreting an empty string as the attribute not existing, and returning defVal even when the attribute did exist and was an empty string. Now it checks explicitly whether the attribute exists. * Deprecated DOMSerializer in favor of javax.xml.transform.Transformer DOMSerializer contains several edge-case issues that were only partially worked around with the encodeString() and decodeString() helper methods. Java has a robust built-in solution to serializing Document objects, and using that instead fixes several bugs. * Fixed NullPointerException when XMLExporter writes a String[] with null Also refactored all primitive array write and read methods to be more readable and reduce duplicate code. * Made DOM capsules reuse write/read primitive array methods for buffers Further reduces duplicate code * Fixed DOMOutputCapsule.write(Savable[][]) NullPointerException Refactored write and read methods for Savables and 1 and 2 dimensional Savable arrays. Fixed NullPointerException when writing a 2d Savable array containing a null element in the outer array. * Added Savable reference consistency test to InputOutputCapsuleTest * Fixed DOMInputCapsule throwing NullPointerException when reading list DOMInputCapsule used to throw a NullPointerException when reading an Arraylist containing a null element. Also refactored list write and read methods to clean up a bit and accidentally also fixed an unrelated bug where reading ArrayList<ByteBuffer> would return a list containing all null elements. * Made XMLExporter save and load buffer positions properly. * Cleanup and formatting for XMLExporter related classes * Undid XMLExporter saving buffer positions. Not saving positions is intentional #2312 (comment) * Fixed infinite recursion with XMLExporter Writing a Savable containing a reference loop caused infinite recursion due to bookkeeping being performed after the recursive call instead of before. Also added a unit test for this to InputOutputCapsuleTest.
ReflectionAllocator uses
java.nio.ByteByffer.allocateDirect()
which always initializes new buffers to all zeros.LWJGLBufferAllocator uses
org.lwjgl.system.MemoryUtil.nmemAlloc()
which does not make the same guarantee. (nmemAlloc doesn't have much documentation, just redirects to this memAlloc description)Strangely, it seems that this discrepancy only starts manifesting after buffers have been destroyed, so it isn't immediately obvious. Since the uninitialized data is overwritten immediately 99% of the time, it's not a big deal, but my understanding is that LWJGLBufferAllocator is intended to be a drop-in replacement for ReflectionAllocator.
Here's an MRE:
Possible fix could be to add a MemoryUtil.memSet() call after nmemAlloc(). I can test it out and make a PR later if y'all want.
The text was updated successfully, but these errors were encountered: