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

Dynamic override question #981

Open
Bvsemmer opened this issue Jan 4, 2025 · 5 comments
Open

Dynamic override question #981

Bvsemmer opened this issue Jan 4, 2025 · 5 comments

Comments

@Bvsemmer
Copy link

Bvsemmer commented Jan 4, 2025

Hi!

I am in the process of doing some experimentation with MiMalloc on Windows and I have a question regarding the dynamic override that MiMalloc provides and the advice to globally override new and delete (with the supplied mimalloc-new-delete.h header).

I always assumed a global override new/delete is bad practice on Windows, since memory allocated in 3rd party dlls, that do not provide a proper memory override hook, will give problems if you free that memory in your own binary. Should the dynamic override somehow prevent this from happening?

I am asking, since I have a very simple app that is using hwinfo as a dependency through vcpkg. vcpkg compiles this library as a dll. This particular library is not really well behaving, since it returns internal memory for the caller to free, but this is besides the point.
I just see that this of course crashes, since hwinfo is not using MiMalloc. Since the allocation happens in the dll, outside of MiMalloc, and the free happens by my library, using MiMalloc.

So, to me this feels using a global override is only possible if you have 100% control over your source code. What am I missing?

@daanx
Copy link
Collaborator

daanx commented Jan 5, 2025

Hi @Bvsemmer -- you are not missing anything; it is tricky to override malloc/free dynamically on Windows since each DLL has its own "namespace". However, almost all DLL's link eventually with the universal C runtime (ucrtbase.dll) for malloc/free (and with ntdll.dll for HeapAlloc). With mimalloc we provide a redirection dll (mimalloc-redirect.dll) which should be loaded before the C runtime and it patches all malloc/free entries to use mimalloc instead. This way, all DLL's and libraries will all be redirected to use mimalloc. This is a tricky process but it has been working well even on very large services.

(however, in the long run, the goal is still to make overriding malloc/free per-process in Windows easier without the need for the mimalloc-redirect dll).

@Bvsemmer
Copy link
Author

Bvsemmer commented Jan 6, 2025

Hi @daanx, thank you for your reply!

Then I must be doing something wrong. I have made a very simple testcase where the redirect is not working, nonetheless.

My main app is linking with MiMalloc and is using the mimalloc-new-delete.h header. I confirmed, any allocations I do in my app pass through MiMalloc.

I have also used minject.exe to make sure, the mimalloc dlls are loaded first. This is the output of minject -l of my exe:
Image

When I launch with MI_VERBOSE=1, I also see that mimalloc is initialized and that malloc is overwritten:
Image

The mimalloc dlls are also loaded:
Image

This is my test case:
I made a very simple DLL project, that exposes a GetString() function with the following test code:

class test
{
public:
	__declspec(dllexport) std::string GetString();
};
std::string test::GetString()
{
	char* test = new char[128];
	memset(test, 0, 128);
	const char* t = "test";
	memcpy(test, t, 4);
	std::string r = test;
	delete[] test;
	return r;
}

My main app, links with this library and calls GetString in it's main function.

int main(int argc, char *argv[])
{
    {
        test t;
        std::string s = t.GetString();
    }
}

When 's' goes out of scope, I crash since std::string is trying to deallocate memory that was not allocated by MiMalloc (in the DLL). I also confirmed this, the allocations in the GetString() function are passing through the standard CRT allocator, when I step through it.

When I put the exact same code in the main body of my app, the allocations happen correctly, as expected, and no crash is happening:

int main(int argc, char *argv[])
{
    {
        char* test = new char[128];
        memset(test, 0, 128);
        const char* t = "test";
        memcpy(test, t, 4);
        std::string r = test;
        delete[] test;
    }
}

Clearly I am overlooking something.

@daanx
Copy link
Collaborator

daanx commented Jan 6, 2025

Hmm, it looks like that should work; I'll try to repro and see what is going on.

@daanx
Copy link
Collaborator

daanx commented Jan 6, 2025

I tried it and for me it just works -- I have checked in your test in the dev and dev2 branches so perhaps you can try yourself too. (open the ide\vs2022\mimalloc.sln solution and set mimalloc-test-override as the startup project.

I guess there is some setup issue -- I see you use mimalloc-override.dll but the latest mimalloc uses mimalloc.dll (together with mimalloc-redirect.dll). If there is still a problem, what is the windows version? and what VS version?

@Bvsemmer
Copy link
Author

Bvsemmer commented Jan 7, 2025

Hey Daan,

I have a suspicion, but I am not sure how to fix it. I am using MiMalloc through vcpkg (version 2.1.7). But there is a chance this is not configured 100% correctly.

When I launch MiMalloc using the default installation, with dynamic override, I get a popup that mimalloc-override.dll is missing. (which seems to be a dependency for mimalloc-redirect.dll)

It noticed the vcpkg port file only builds mimalloc.dll and mimalloc-redirect.dll, not mimalloc-override.dll. Even though the mimalloc override project is included in the Solution that is part of the source tree.

Image

So, I figured to build it myself and copy and manage the dll myself. Probably there is a mismatch, even though MiMalloc seems to launch just fine now.

Is the vcpkg version of MiMalloc officially maintained? Here I noticed the last update was 2 months ago, so seemed recent enough for me.

Thank you for the swift replies!

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

No branches or pull requests

2 participants