Skip to content
This repository has been archived by the owner on Apr 15, 2023. It is now read-only.

Destructors #72

Closed
wants to merge 5 commits into from
Closed

Destructors #72

wants to merge 5 commits into from

Conversation

greenfork
Copy link
Owner

@greenfork greenfork commented Feb 7, 2022

Currently crashes on exit. If I remove closeWindow(), doesn't crash. Probably because there's a call to unloadTexture after the window was closed and that causes the crash.

$ cd examples/textures/
$ nim r --gc:orc textures_to_image.nim
Hint: used config file '/home/grfork/.choosenim/toolchains/nim-1.6.2/config/nim.cfg' [Conf]
Hint: used config file '/home/grfork/.choosenim/toolchains/nim-1.6.2/config/config.nims' [Conf]
..........................................................................................
/home/grfork/reps/nimraylib_now/src/nimraylib_now/static_build.nim(4, 1) Hint: duplicate import of 'os'; previous import here: /home/grfork/reps/nimraylib_now/src/nimraylib_now/mangled/raylib.nim(6, 6) [DuplicateModuleImport]
.........
CC: stdlib_dollars.nim
CC: ../../src/nimraylib_now/mangled/raylib.nim
Hint:  [Link]
Hint: gc: orc; opt: none (DEBUG BUILD, `-d:release` generates faster code)
53442 lines; 0.713s; 75.309MiB peakmem; proj: /home/grfork/reps/nimraylib_now/examples/textures/textures_to_image.nim; out: /home/grfork/.cache/nim/textures_to_image_d/textures_to_image_5FC9DD2C66B4B8B494B4BD9E4BF3F178F0C7F7DD [SuccessX]
Hint: /home/grfork/.cache/nim/textures_to_image_d/textures_to_image_5FC9DD2C66B4B8B494B4BD9E4BF3F178F0C7F7DD  [Exec]
INFO: Initializing raylib 4.0
INFO: DISPLAY: Device initialized successfully
INFO:     > Display size: 1920 x 1080
INFO:     > Screen size:  800 x 450
INFO:     > Render size:  800 x 450
INFO:     > Viewport offsets: 0, 0
INFO: GLAD: OpenGL extensions loaded successfully
INFO: GL: Supported extensions count: 229
INFO: GL: OpenGL device information:
INFO:     > Vendor:   AMD
INFO:     > Renderer: AMD RENOIR (DRM 3.44.0, 5.16.5-arch1-1, LLVM 13.0.0)
INFO:     > Version:  4.6 (Core Profile) Mesa 21.3.5
INFO:     > GLSL:     4.60
INFO: GL: DXT compressed textures supported
INFO: GL: ETC2/EAC compressed textures supported
INFO: TEXTURE: [ID 1] Texture loaded successfully (1x1 | R8G8B8A8 | 1 mipmaps)
INFO: TEXTURE: [ID 1] Default texture loaded successfully
INFO: SHADER: [ID 1] Vertex shader compiled successfully
INFO: SHADER: [ID 2] Fragment shader compiled successfully
INFO: SHADER: [ID 3] Program shader loaded successfully
INFO: SHADER: [ID 3] Default shader loaded successfully
INFO: RLGL: Render batch vertex buffers loaded successfully in RAM (CPU)
INFO: RLGL: Render batch vertex buffers loaded successfully in VRAM (GPU)
INFO: RLGL: Default OpenGL state initialized successfully
INFO: TEXTURE: [ID 2] Texture loaded successfully (128x128 | GRAY_ALPHA | 1 mipmaps)
INFO: FONT: Default font loaded successfully (224 glyphs)
INFO: FILEIO: [resources/raylib_logo.png] File loaded successfully
INFO: IMAGE: Data loaded successfully (256x256 | R8G8B8 | 1 mipmaps)
INFO: TEXTURE: [ID 3] Texture loaded successfully (256x256 | R8G8B8 | 1 mipmaps)
INFO: TEXTURE: [ID 3] Pixel data retrieved successfully
INFO: TEXTURE: [ID 4] Texture loaded successfully (256x256 | R8G8B8 | 1 mipmaps)
INFO: TEXTURE: [ID 3] Unloaded texture data from VRAM (GPU)
INFO: TEXTURE: [ID 2] Unloaded texture data from VRAM (GPU)
INFO: SHADER: [ID 3] Default shader unloaded successfully
INFO: TEXTURE: [ID 1] Default texture unloaded successfully
INFO: Window closed successfully
Traceback (most recent call last)
/home/grfork/reps/nimraylib_now/src/nimraylib_now/mangled/raylib.nim(3103) textures_to_image
/home/grfork/reps/nimraylib_now/src/nimraylib_now/mangled/raylib.nim(3104) =destroy
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Error: execution of an external program failed: '/home/grfork/.cache/nim/textures_to_image_d/textures_to_image_5FC9DD2C66B4B8B494B4BD9E4BF3F178F0C7F7DD '

@planetis-m
Copy link
Collaborator

Yes exactly, so either put closeWindow in a try/finally or defer or let the user encapsulate in a destructor backed object.

@greenfork
Copy link
Owner Author

Yes exactly, so either put closeWindow in a try/finally or defer

On top-level defers are forbidden. It might be an okay idea to wrap every example in main.

or let the user encapsulate in a destructor backed object.

I don't think it is a nice idea to say that to the user. The users of this library might be of beginner level.

Currently another problem is that I'm not sure that we have a complete list of all possible destructors. We can't say to the user "here is a list of types you need to memorize, you don't need to unload them; here is another list which you should unload manually".

unloadVrStereoConfig I removed it, its a noop for now.

That is fine, we can add it later once it has implementation.

unloadWaveSamples just a for loop that calls unloadWaveSample, since we used seq[Wave] it was not need.
unloadImageColors, unloadFontData, unloadImagePalette I think same as above

  1. It's not entirely clear that we want a seq type here.
  2. It's not obvious that Nim would manage the memory that was initialized outside of Nim. We can check it of course but I'm afraid of corner cases.

unloadFileData, unloadFileText it's hard to tell what to do with these, could we instead use std equivalents instead of LoadFile*

It is okay to use the std variant but we probably still want a destructor for it so that --gc:orc is going to manage the memory in this case.

unloadCodepoints haven't decided what to do with codepoints yet.

This can wait for sure.

@planetis-m
Copy link
Collaborator

Currently another problem is that I'm not sure that we have a complete list of all possible destructors.

There is one I left out... ClearDroppedFiles and I don't think anyone has yet.

@greenfork greenfork closed this Apr 15, 2023
@greenfork greenfork deleted the destructors branch April 15, 2023 05:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants