Undo script patch on eject.

[gir489returns]

Closes #2887

[gir489returns]

MBG has been silent for a week now, and I've asked him multiple times to look at the eject crash. I tried to find out why it's crashing for myself, and I can't figure it out. If I attach a debugger to GTA V when it's crashing, it says it has an access violation on some native called from the main function which just tells me we're doing something while the game is trying to do something.

This commit reverses each script patch when the destructor is called, however, the crash from ejecting/reinjecting is still present.

[github-actions]

Download the artifacts for this pull request:

• binary.zip

[Rxann]

Would a mutex fix the issue? Just something I thought of.

[gir489returns]

Would a mutex fix the issue? Just something I thought of.

No, the game itself is crashing for unknown reasons. If we could control a mutex from the game, that would help, but it doesn't use one for the main thread.

[Rxann]

Would a mutex fix the issue? Just something I thought of.

No, the game itself is crashing for unknown reasons. If we could control a mutex from the game, that would help, but it doesn't use one for the main thread.

Okay, that does make sense. You said above we were accessing something the game was aswell, so could we just virtualprotect to block the game from trying to do what it needs to do at the address while we do it?

[gir489returns]

Would a mutex fix the issue? Just something I thought of.

No, the game itself is crashing for unknown reasons. If we could control a mutex from the game, that would help, but it doesn't use one for the main thread.

Okay, that does make sense. You said above we were accessing something the game was aswell, so could we just virtualprotect to block the game from trying to do what it needs to do at the address while we do it?

No, that was a shot in the dark. It only crashes "sometimes", so it stands to reason, that whatever we're doing only causes the crash "sometimes" due to multithreading. If it always occurred, it would have to be because of something we're directly doing. I believe the reason has something to do with the timing. Sometimes I can get it to occur after 3 inject/reinjects, sometimes I can never get it to occur.

[rkwapisz]

I pushed these commits into my build for some testing since I've also had similar random crashes when unloading YimMenu.
My latest crash with these commits in my build is as follows. I am not sure if they are related, but since we're on the topic of unloading the menu...

Dumping stacktrace:
[0]	VCRUNTIME140.dll memmove
[1]	YimMenu.dll big::lua_patch::`scalar deleting destructor'
[2]	D:\Documents\GitHub\YimMenu\src\lua\lua_module.cpp L: 136 big::lua_module::~lua_module
[3]	D:\Documents\GitHub\YimMenu\src\lua\lua_manager.cpp L: 108 big::lua_manager::unload_all_modules
[4]	D:\Documents\GitHub\YimMenu\src\lua\lua_manager.cpp L: 42 big::lua_manager::~lua_manager
[5]	C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\memory L: 3283 std::unique_ptr<big::lua_manager,std::default_delete<big::lua_manager> >::reset
[6]	D:\Documents\GitHub\YimMenu\src\main.cpp L: 289 `DllMain'::`5'::<lambda_1>::operator()
[7]	D:\Documents\GitHub\YimMenu\src\main.cpp L: 347 `DllMain'::`5'::<lambda_1>::<lambda_invoker_cdecl>
[8]	KERNEL32.DLL BaseThreadInitThunk
[9]	ntdll.dll RtlUserThreadStart
--------End of exception--------

[gir489returns]

@xiaoxiao921 Could the crash be caused by something Lua is doing during its cleanup phase?

[Rxann]

Looking at that, in the lua_module destructor, it seems to be deleting memory. This may have something to do with it, just thought I would share:

Line 134, lua_module.cpp

for (auto memory : m_allocated_memory)
			delete[] memory;
			 

[rkwapisz]

Looking at that, in the lua_module destructor, it seems to be deleting memory. This may have something to do with it, just thought I would share:

Line 134, lua_module.cpp

for (auto memory : m_allocated_memory)
			delete[] memory;
			 

Yes, my initial guess is that the destructor is not inherently safe since we're not checking for null pointers before trying to delete blocks of memory. I've been testing loading/unloading the menu with the following change, so far without issues:

for (auto memory : m_allocated_memory) { if (memory != nullptr) { delete[] memory; memory = nullptr; } }

[xiaoxiao921]

Well the code is fine. Could you tell me how it is wrong?
https://github.com/search?q=repo%3AYimMenu%2FYimMenu%20m_allocated_memory&type=code

[gir489returns]

Looking at that, in the lua_module destructor, it seems to be deleting memory. This may have something to do with it, just thought I would share:
Line 134, lua_module.cpp

for (auto memory : m_allocated_memory)
			delete[] memory;
			 

Yes, my initial guess is that the destructor is not inherently safe since we're not checking for null pointers before trying to delete blocks of memory. I've been testing loading/unloading the menu with the following change, so far without issues:

for (auto memory : m_allocated_memory) { if (memory != nullptr) { delete[] memory; memory = nullptr; } }

This makes no sense.

[xiaoxiao921]

Indeed, if anything you are calling pointer.free twice and in this case this is simply a skill issue.

[rkwapisz]

Looking at that, in the lua_module destructor, it seems to be deleting memory. This may have something to do with it, just thought I would share:
Line 134, lua_module.cpp

for (auto memory : m_allocated_memory)
			delete[] memory;
			 

Yes, my initial guess is that the destructor is not inherently safe since we're not checking for null pointers before trying to delete blocks of memory. I've been testing loading/unloading the menu with the following change, so far without issues:
for (auto memory : m_allocated_memory) { if (memory != nullptr) { delete[] memory; memory = nullptr; } }

This makes no sense.

Understood. I thought a double-check was prudent, because otherwise I cannot understand why delete[] was being considered scalar.

[gir489returns]

I cannot understand why delete[] was being considered scalar.

What in the absolute C++20 standard are you talking about?!

[maybegreat48]

I've been looking into this, and all I've found out so far is that manually ejecting script patches wouldn't fix the issue since the code pages are hotswapped at the script VM. So there wouldn't be any permanent damage even if the patches aren't cleaned for some reason. A reasonable guess is that the instruction pointer gets corrupted when you load the menu and end up patching the middle of an instruction, but this is all unconfirmed speculation

[xiaoxiao921]

Makes sense, so a fix would be to make sure the vm is not currently executing something we are about to patch, my gut feeling would be to suspend the thread that the vm runs on and observe its instruction pointer and making sure it's not near any of the stuff that we patch