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

Calling MemFree after an explicit destruction of the last context #992

Closed
aiekick opened this issue Jan 22, 2017 · 13 comments
Closed

Calling MemFree after an explicit destruction of the last context #992

aiekick opened this issue Jan 22, 2017 · 13 comments

Comments

@aiekick
Copy link
Contributor

aiekick commented Jan 22, 2017

Hello,

i have a InputText Field, if i copy paste some data from the clipboard. when i call the app, after the ImGui::Shutdown(); the app crash here in imgui.cpp
screenhunter_110 jan 22 22 03

i use your imgui_impl_sdl of example directory.

if i write something, there is no problem, but when i do ctrl+v, it crash (only when i close the app).
I dont know why this code is called after the ImGui:Shutdown(), and the callgraph of vs2015 on win7 give that
screenhunter_111 jan 22 22 06

@ocornut
Copy link
Owner

ocornut commented Jan 22, 2017

Can you confirm that you are getting the bug when running the unmodified sample from the sdl_opengl2_example folder?

There's no code afaik that explicitly set GImGui to NULL.
EDIT Actually, DestroyContext may set GImGui to NULL if it is the current context, it is however not called by any example code.

@aiekick
Copy link
Contributor Author

aiekick commented Jan 22, 2017

no it work, its weird, i have replace all the imgui and the interface with sdl2 from my app in the example and it work.
it may be a problem in my windows system..
i create a imgui context and i destroy it, maybe like you mention, it is due to that.

but im not sure why i created a context :), i find that in a sample on the net. what is the goal to create a imgui context ?

@aiekick
Copy link
Contributor Author

aiekick commented Jan 22, 2017

it was for a multi windows system. how can i destroy the current context properly ? it is necessary ?

@ocornut
Copy link
Owner

ocornut commented Jan 23, 2017

Hmm.

Right now the easiest workaround is to never destroy the initial/default context.
You have memory (probably a static ImVector) allocated that's being free after the context is destroyed, it is a restriction that's inconvenient and undesirable. I'm not often running into it myself as I don't really use multiple context.

It's an issue with the design of storing those MemAllocFn MemFreeFn handlers in the context.
Perhaps they should just be global in the first place, as it is unlikely that people would need different allocator for different contexts (and even so, we could add a context pointer to the allocator function?).

I chose to put them in the context to avoid relying on #define which are less obvious to setup depending on your build system and/or how/if you compile dear imgui yourself.

I think I might just bite the bullet and move to global functions, aka GImGuiMalloc, GImGuiFree ?

Moving this discussion to #586

@aiekick
Copy link
Contributor Author

aiekick commented Jan 24, 2017

for the moment, i not used the many context. and the bug appear only with the use of ctrl+v, so for the moment, i can wait a fix :) thanks, for the explains

@ocornut ocornut changed the title The app crash at close if i use ctrl+v in a InputText (multiline or not) Calling MemFree after an explicit destruction of the last context Jan 24, 2017
@sonoro1234
Copy link

It is not happening for destroying default context, just destroying any context sets GImGui to NULL.

@MrSapps
Copy link

MrSapps commented Feb 6, 2017

How come unmodified sample hasn't got this issue then?

@sonoro1234
Copy link

unmodified sample only uses default context

@zx64
Copy link

zx64 commented Nov 10, 2017

I've been running into a sporadic crash on shutdown in a Windows codebase that uses a non-default context. The code lives inside a DLL, so context lifetime is tied to when the DLL loads and unloads, but static variables are only destructed at process exit which happens after the DLL is unloaded...

The callstack points at the destructor for this variable:

static const char* GetClipboardTextFn_DefaultImpl(void*)
{
    static ImVector<char> buf_local;

I've locally changed it so it always uses a ImVector<char> that's tied to the current context:
ImVector<char>& buf_local = GImGui->PrivateClipboard;

I haven't done substantial testing to confirm if that fixes my problem, but does that seem like a reasonable change to make?

@ocornut
Copy link
Owner

ocornut commented Nov 11, 2017

@zx64 Well it would work but it is a little odd and misleading. That clipboard function should have nothing to do with an imgui context and you are using a buffer than happens to be lying in a context, so it ties the lifetime of the return value to the context. Practically speaking it is probably ok but it feels very misleading.

The code lives inside a DLL, so context lifetime is tied to when the DLL loads and unloads, but static variables are only destructed at process exit which happens after the DLL is unloaded...

If I understand correctly the problem is that at the time of destruction ImVector<> cannot access a destructor since it is stored in the current GImGui instance pointer?
Would my suggested changes in this message #586 (comment) solve the problem?

If that's the case you might also be able to temporarily workaround by restoring the default context after destroying your context.

(I suppose I should bite the bullet at some point and setup a DLL system to understand those problem better - I'm happy using Edit & Continue or Recode which doesn't requires DLL nor kill instances).

@zx64
Copy link

zx64 commented Nov 11, 2017

I think those changes would help - it'd avoid having ImVector's internal storage being allocated by one context and freed by another.
The asymmetric lifetime of function statics makes this especially awkward, hence my initial thought to remove needing to have a static variable - and there happened to be a buffer used by the non-Win32 default implementation of the clipboard.

@ocornut
Copy link
Owner

ocornut commented Nov 11, 2017 via email

ocornut added a commit that referenced this issue Jan 20, 2018
…now setup with SetMemoryAllocators() and shared by all contexts. (#586, #992, #1007, #1558)
ocornut added a commit that referenced this issue Jan 21, 2018
…now setup with SetMemoryAllocators() and shared by all contexts. (#1565, #586, #992, #1007, #1558)
ocornut added a commit that referenced this issue Jan 21, 2018
- YOU NOW NEED TO CALL ImGui::CreateContext() AT THE BEGINNING OF YOUR APP, AND CALL ImGui::DestroyContext() AT THE END.
- removed Shutdown() function, as DestroyContext() serve this purpose.
- you may pass a ImFontAtlas* pointer to CreateContext() to share a font atlas between contexts. Otherwhise CreateContext() will create its own font atlas instance.
- removed allocator parameters from CreateContext(), they are now setup with SetAllocatorFunctions(), and shared by all contexts.
- removed the default global context and font atlas instance, which were confusing for users of DLL reloading and users of multiple contexts
(#1565, #586, #992, #1007, #1558)
ocornut added a commit that referenced this issue Jan 21, 2018
…ntext(), DestroyContext() in example code. Removed Shutdown() from binding code. (#1565, #586, #992, #1007, #1558)
@ocornut
Copy link
Owner

ocornut commented Feb 6, 2018

@aiekick the changes described in #1565 should fix everything described in this issue.
You can now override the allocators with ImGui::SetAllocatorFunctions(), and they aren't stored in the context at all.

There's no default static/global context in imgui.cpp, only a pointer and you need to call CreateContext() to allocate a context.

If you have ImVector<> etc. destructed during program destruction when you'd want your allocators to be available during destruction as well.

@ocornut ocornut closed this as completed Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants