Skip to content

Fix crash in state restoration with multiple of 32 fields#707

Merged
rossbacher merged 2 commits intomainfrom
fixStatePersistenceWithMultiplesOf32Parameters
Jan 25, 2024
Merged

Fix crash in state restoration with multiple of 32 fields#707
rossbacher merged 2 commits intomainfrom
fixStatePersistenceWithMultiplesOf32Parameters

Conversation

@rossbacher
Copy link
Copy Markdown
Collaborator

If the field count is exactly multiples of 32 the bitmap count would be be too high (e.g. 2 with field count 32 and 3 with field count 64) leading to a crash when calling the copy function via reflection.

Added test that tests that case.

… be be too high (e.g. 2 with field count 32 and 3 with field count 64) leading to a crash when calling the copyt function via reflection.

Added test that tests that case.
Copy link
Copy Markdown
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the fix and test 🙏

@Test
fun testClassWithExactly32Parameters() {
data class StateWith32Params(
val p0: Int = 0,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it matter that there are only 16 persist state annotations here? it's confusing why only half of the params have it, but I suppose it shouldn't matter for the purposes of this bug?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the number of parameters in the constructor that trigger the bug not the number of persisted elements, I copied this from another test, that is why this is like this, but we could also persist them all or persist none of them.
The test was failing before the fix.


// There is 1 bitmask for each block of 32 parameters.
val parameterBitMasks = IntArray(fieldCount / 32 + 1) { 0 }
val parameterBitMasks = IntArray(fieldCount / 32 + if (fieldCount % 32 != 0) 1 else 0) { 0 }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be simpler to round up to the next int ceil(fieldCount/32.0).toInt()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was what I was looking for yesterday but could not come up with it. I'll update it.

@rossbacher rossbacher merged commit 68e4869 into main Jan 25, 2024
@rossbacher rossbacher deleted the fixStatePersistenceWithMultiplesOf32Parameters branch January 25, 2024 21:01
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

Successfully merging this pull request may close these issues.

3 participants