Skip to content

fix: 32bit-ARMv7 crashes on android#2654

Merged
NoelStephensUnity merged 24 commits intodevelopfrom
fix/32bit-custom-named-message-crash-android
Aug 8, 2023
Merged

fix: 32bit-ARMv7 crashes on android#2654
NoelStephensUnity merged 24 commits intodevelopfrom
fix/32bit-custom-named-message-crash-android

Conversation

@NoelStephensUnity
Copy link
Copy Markdown
Member

This PR resolves the issue with ARMv7 builds crashing on the Android.

fix: #2646

Changelog

  • Fixed: Issue where ARMv7 Android builds would crash when trying to validate the batch header.

Testing and Documentation

  • Includes integration tests (?)
  • No documentation changes or additions were necessary.

Temporarily commenting out the batch header hash check to have a user validate that this fixes the issue with Android Armv7 builds.
This resolves the issue with XXHash causing ARMv7 builds to crash when validating the batched message header and payload.
Keep messages sent word aligned.
Use the 32bit version of XXHash.
Prevent padding on the NetworkBatchHeader
Comment thread com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs Outdated
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review August 7, 2023 21:16
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner August 7, 2023 21:16
NoelStephensUnity and others added 3 commits August 7, 2023 16:40
fixing whitespace issue with the other test changes.
This resolves the tools bytes measured tests.
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner August 8, 2023 16:16
NoelStephensUnity and others added 5 commits August 8, 2023 11:41
This resolves the tools bytes measured tests.
switching back to 64 bit hash values for batch header validation.
Comment thread com.unity.netcode.gameobjects/Tests/Runtime/Metrics/TransportBytesMetricsTests.cs Outdated
reverting the switch to 32 bit hash values.
reverting the conditional registration of either 32bit or 64 bit hash values in either table to just register in both tables.
 reverting the additional CR/LF
reverting the code to be exactly as it was (no need to make organizational changes)
removing the alignment calculation from the observed byte count.
Removing additional CR/LFs.
removing extra LF/CRs.
@simon-lemay-unity
Copy link
Copy Markdown
Contributor

Will let you decide if you think it's worth the hit to readability, but you can calculate aligned lengths this way:

alignedLength = (length + 7) & ~7;

It avoids dealing with floating points and only uses fast integer instructions.

@ShadauxCat
Copy link
Copy Markdown
Collaborator

Will let you decide if you think it's worth the hit to readability, but you can calculate aligned lengths this way:

alignedLength = (length + 7) & ~7;

It avoids dealing with floating points and only uses fast integer instructions.

Oh good point. +1 to this @NoelStephensUnity

Applying Simon's suggestion.
This update assures we won't go above the MTU size due to word alignment.
removing VS 2022 auto-assigned namespace...<grr>
@NoelStephensUnity NoelStephensUnity merged commit cbc2d72 into develop Aug 8, 2023
@NoelStephensUnity NoelStephensUnity deleted the fix/32bit-custom-named-message-crash-android branch August 8, 2023 21:11
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.

Game crashes when joining on Android 11?

5 participants