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

Fixed a crash #194

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Fixed a crash #194

merged 3 commits into from
Aug 16, 2024

Conversation

RaphaelIT7
Copy link
Contributor

[#] Fixed a crash

Related issues:
fixes #137
fixes #85

[#] Fixed a crash

Related issues:
fixes misyltoad#137
fixes misyltoad#85
@misyltoad
Copy link
Owner

Let's come back to this after 0.20, do you have a repro case?

@RaphaelIT7
Copy link
Contributor Author

I'll can get one in a few hours

@YUCLing
Copy link

YUCLing commented Aug 15, 2024

Let's come back to this after 0.20, do you have a repro case?

Maybe check the related issues? It looks like it's simple to repro in GMod with an expanding hydraulic.

@RaphaelIT7
Copy link
Contributor Author

Latest Build:

crash.mp4

@RaphaelIT7
Copy link
Contributor Author

I did a bit more testing, and it seems like JoltPhysicsSpring::OnJoltPhysicsObjectDestroyed will be called and then it breaks.
If a prop is connected to the world, it will crash in JoltPhysicsSpring::SetSpringLength because the m_pObjectStart was deleted, but there is no null check so it'll try to use it.
I'll try to test it future again tomorrow, but it seems like the crash in the video is a different one?
(I don't remember it ever crashing in one of the JoltPhysicsSpring functions when I made the pr)

@misyltoad
Copy link
Owner

Is it not set up to have an OnDestroyedListener?

@RaphaelIT7
Copy link
Contributor Author

RaphaelIT7 commented Aug 15, 2024

It has and OnJoltPhysicsObjectDestroyed will be called to set m_pObjectStart to nullptr but it's still used / the engine still calls SetSpringLength and in that function you don't check if it's null

@misyltoad
Copy link
Owner

I guess MR that NULL check? :-)

@RaphaelIT7
Copy link
Contributor Author

RaphaelIT7 commented Aug 15, 2024

It'll solve the world -> prop crash but not the prop -> prop crash sadly

I'll look into it tomorrow again to try and solve it

@RaphaelIT7
Copy link
Contributor Author

So It seems like I found the cause, and it should now be fixed for all cases :D

I also reverted the original two commits, since it seems like they didn't really do anything.
I wonder why they fixed it on the dupe months ago.

@misyltoad misyltoad merged commit 9f3bff4 into misyltoad:main Aug 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants