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

__attribute__((destructor)) is a bad place for cleanup, so xr_3da fails assert on termination #804

Closed
kosumosu opened this issue May 24, 2021 · 9 comments
Labels
Bug The issue in the run-time. Linux Portability
Milestone

Comments

@kosumosu
Copy link
Contributor

kosumosu commented May 24, 2021

I faced several failed assertions in mimalloc when I was trying to close the game.

I started investigating this and found out that the problem was not in the allocator itself, but in the fact that the allocator was destroyed BEFORE the failed assertion. Further investigation has shown that even though SDL_UnloadObject() is called before returning from main(), the __attribute__((destructor)) unload() is called AFTER return from main(). So global variables inside xrGame.so turn into a garbage by the time the control reaches unload().
This causes xray to crash every time it terminates on e2k. It also leads to hanging fullscreen, causing big trouble to a user.

Although a dlclose() operation is not required to remove structures from an address space, neither is an implementation prohibited from doing so.
https://pubs.opengroup.org/onlinepubs/007904975/functions/dlclose.html

So resources from shared libraries must be cleaned up explicitly instead.

@kosumosu kosumosu added the Bug The issue in the run-time. label May 24, 2021
@kosumosu
Copy link
Contributor Author

kosumosu commented May 24, 2021

This is mainly Linux-related. And in theory, not only on Elbrus.

@Xottab-DUTY Xottab-DUTY added this to the Linux port milestone May 30, 2021
Xottab-DUTY added a commit that referenced this issue May 30, 2021
… __attribute__((constructor)) / __attribute__((destructor)) (#804)
@Xottab-DUTY
Copy link
Member

@kosumosu, I just pushed 810692a to fix this problem. Please, test it. :)

Also, can you test it with disabled workaround for GCC in FACTORY_PTR_INSTANCIATE in src/Include/xrRender/FactoryPtr.h?

@kosumosu
Copy link
Contributor Author

kosumosu commented Jun 3, 2021

I confirm, the game doesn't crash anymore 👍

@kosumosu kosumosu closed this as completed Jun 3, 2021
@Xottab-DUTY
Copy link
Member

@kosumosu doesn't crash even with removed FACTORY_PTR_INSTANCIATE workaround?

@kosumosu
Copy link
Contributor Author

kosumosu commented Jun 3, 2021

I'm trying this right now...

@kosumosu
Copy link
Contributor Author

kosumosu commented Jun 3, 2021

@Xottab-DUTY Without it I got SIGSEGV when tried to "exit to windows" from menu right away.

1 CUIStatsIcon::TEX_INFO::~TEX_INFO()   0x4555b0442c58 
2 __cxa_vec_dtor                        0x455558421338 
3 __run_exit_handlers                   0x455558957d68 
4 exit                                  0x4555589587f0 
5 __libc_start_main                     0x4555588f5048 
6 _start                                0x1c1e0        

@Xottab-DUTY
Copy link
Member

@kosumosu, hi! Could you please try to remove FACTORY_PTR_INSTANCIATE workaround in src/Include/xrRender/FactoryPtr.h again and test it?
In a914f65 I've changed how CUIStatsIcon::TEX_INFO works, so it might help with that.

@kosumosu
Copy link
Contributor Author

kosumosu commented May 8, 2024

That have been a long time. I'm not sure I still have access to e2k and have proper setup...

@Xottab-DUTY
Copy link
Member

@kosumosu, this should be reproducible on any Linux machine I think.

Xottab-DUTY added a commit that referenced this issue May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug The issue in the run-time. Linux Portability
Projects
Status: Done
Development

No branches or pull requests

2 participants