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

libMagnumGlxContext.a shared relocation error #90

Closed
Bazilikum opened this Issue Feb 20, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@Bazilikum

Bazilikum commented Feb 20, 2015

Hello,
the error i get now after updating to latest version (long time after last update for me):
/usr/lib/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../lib/libMagnumGlxContext.a(flextGLPlatform.cpp.o): relocation R_X86_64_32 against `.rodata' can not be used when making a shared object; recompile with -fPIC
Obviously Im on linux compiling on gcc4.9
I noticed you changed the method for custom toolkit context creation (Platform::Context) and the need to find GlxContext component in my cmake file
I also compile Magnum with:
GLXAPPLICATION
GLXCONTEXT
WINDOWLESSGLXAPPLICATION
I tried building Magnum with static_pic but it doesnt help.

I tried without new context component request and not linking against GlxContext but then i get a linker error:
undefined ... 'flextGLInit'

Thank you :)
Amit

@Bazilikum

This comment has been minimized.

Show comment
Hide comment
@Bazilikum

Bazilikum Feb 28, 2015

i fixed that by forcing cmake to build PIC static libraries for all corrade and magnum libs
(SET(CMAKE_POSITION_INDEPENDENT_CODE ON))
but then I ran into another error:
I staticly link magnum to my main shared library - lets call it 'a.so'
then another shared library is linked to my main library a.so
magnum.a -> static -> a.so
b.so -> dynamic -> a.so
My program uses both a.so and b.so
i use Magnum/context.h in both a.so and b.so
upon program exit i get double-linked list corruption error, even without creating a magnum context.
i assumed there is a global variable set somewhere in magnum that gets destroyed 2 times when the libraries are unloaded and found DefaultFrameBuffer is set that way (Magnum::defaultFrameBuffer)
I tried wrapping it so:
DefaultFramBuffer& defaultFrameBuffer(){
static DefaultFrameBuffer _dfb;
return _dfb;
}
and fixing the relevant parts in the relevant cpp's but that didnt solve the problem.
i am by no means an expert, and maybe thats not the right way to solve the global variable double destruction error. It could also be something completely different and i had the wrong idea.
any help would be appreciated :)
Thanks,
Amit.

Bazilikum commented Feb 28, 2015

i fixed that by forcing cmake to build PIC static libraries for all corrade and magnum libs
(SET(CMAKE_POSITION_INDEPENDENT_CODE ON))
but then I ran into another error:
I staticly link magnum to my main shared library - lets call it 'a.so'
then another shared library is linked to my main library a.so
magnum.a -> static -> a.so
b.so -> dynamic -> a.so
My program uses both a.so and b.so
i use Magnum/context.h in both a.so and b.so
upon program exit i get double-linked list corruption error, even without creating a magnum context.
i assumed there is a global variable set somewhere in magnum that gets destroyed 2 times when the libraries are unloaded and found DefaultFrameBuffer is set that way (Magnum::defaultFrameBuffer)
I tried wrapping it so:
DefaultFramBuffer& defaultFrameBuffer(){
static DefaultFrameBuffer _dfb;
return _dfb;
}
and fixing the relevant parts in the relevant cpp's but that didnt solve the problem.
i am by no means an expert, and maybe thats not the right way to solve the global variable double destruction error. It could also be something completely different and i had the wrong idea.
any help would be appreciated :)
Thanks,
Amit.

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 1, 2015

Owner

Hi,

sorry for the late reply, I somehow completely missed this ...

For the PIC error, my original assumption was that one would always link the *Context and *Application static libraries to the final executable and not to any "intermediate" dynamic lib, in which case the Position-Independent-Code flag wouldn't be needed. I'll fix that shortly.

For the double deletion crash, do you have any small repro code which I could use for investigating the issue?

Thanks.

Owner

mosra commented Mar 1, 2015

Hi,

sorry for the late reply, I somehow completely missed this ...

For the PIC error, my original assumption was that one would always link the *Context and *Application static libraries to the final executable and not to any "intermediate" dynamic lib, in which case the Position-Independent-Code flag wouldn't be needed. I'll fix that shortly.

For the double deletion crash, do you have any small repro code which I could use for investigating the issue?

Thanks.

@mosra mosra self-assigned this Mar 1, 2015

@Bazilikum

This comment has been minimized.

Show comment
Hide comment
@Bazilikum

Bazilikum Mar 1, 2015

Hello,

Thanks for the reply, no worries :)
What I think could help is if when setting static pic on CMake for magnum
then that will also pass to the context and other special hard coded static
libraries. I think it doesnt pass down to them.

About the deletion error, i think I traced it down to
Context.cpp 547.
Commenting out this line seems to solve the error for now.
Maybe theres somthing staticly generated in Utility::String?
It fits becuase Valgrind comes down do a std::string destruction error:

==2204== Invalid read of size 4
==2204== at 0x7BA8947: std::basic_string<char, std::char_traits,
std::allocator >::~basic_string() (in /usr/lib/libstdc++.so.6.0.20)

==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x512EDC2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libSceneGraph_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: _run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204== Address 0x100ff8d0 is 16 bytes inside a block of size 28 free'd
==2204== at 0x4C2B103: operator delete(void
) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x4E6DFE2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libRenderer_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: _run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204==
==2204== Invalid free() / delete / delete[] / realloc()
==2204== at 0x4C2B103: operator delete(void
) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x512EDC2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libSceneGraph_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: __run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204== Address 0x100ff8c0 is 0 bytes inside a block of size 28 free'd
==2204== at 0x4C2B103: operator delete(void*) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x4E6DFE2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libRenderer_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: __run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204==

==2204==

If this rings a bell for you let me know.
I will keep digging into this to find the culprit :)
Thanks,
Amit.

On Sun, Mar 1, 2015 at 3:11 PM, Vladimír Vondruš notifications@github.com
wrote:

Hi,

sorry for the late reply, I somehow completely missed this ...

For the PIC error, my original assumption was that one would always link
the *Context and *Application static libraries to the final executable
and not to any "intermediate" dynamic lib, in which case the
Position-Independent-Code flag wouldn't be needed. I'll fix that shortly.

For the double deletion crash, do you have any small repro code which I
could use for investigating the issue?

Thanks.


Reply to this email directly or view it on GitHub
#90 (comment).

Bazilikum commented Mar 1, 2015

Hello,

Thanks for the reply, no worries :)
What I think could help is if when setting static pic on CMake for magnum
then that will also pass to the context and other special hard coded static
libraries. I think it doesnt pass down to them.

About the deletion error, i think I traced it down to
Context.cpp 547.
Commenting out this line seems to solve the error for now.
Maybe theres somthing staticly generated in Utility::String?
It fits becuase Valgrind comes down do a std::string destruction error:

==2204== Invalid read of size 4
==2204== at 0x7BA8947: std::basic_string<char, std::char_traits,
std::allocator >::~basic_string() (in /usr/lib/libstdc++.so.6.0.20)

==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x512EDC2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libSceneGraph_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: _run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204== Address 0x100ff8d0 is 16 bytes inside a block of size 28 free'd
==2204== at 0x4C2B103: operator delete(void
) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x4E6DFE2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libRenderer_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: _run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204==
==2204== Invalid free() / delete / delete[] / realloc()
==2204== at 0x4C2B103: operator delete(void
) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x512EDC2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libSceneGraph_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: __run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204== Address 0x100ff8c0 is 0 bytes inside a block of size 28 free'd
==2204== at 0x4C2B103: operator delete(void*) (vg_replace_malloc.c:507)
==2204== by 0x834614E: __cxa_finalize (in /usr/lib/libc-2.21.so)
==2204== by 0x4E6DFE2: ??? (in
/home/bazilikum/TheOven/bin_d/lib/libRenderer_d.so)
==2204== by 0x400F7F6: _dl_fini (in /usr/lib/ld-2.21.so)
==2204== by 0x8345DB1: __run_exit_handlers (in /usr/lib/libc-2.21.so)
==2204== by 0x8345E04: exit (in /usr/lib/libc-2.21.so)
==2204== by 0x8330806: (below main) (in /usr/lib/libc-2.21.so)
==2204==

==2204==

If this rings a bell for you let me know.
I will keep digging into this to find the culprit :)
Thanks,
Amit.

On Sun, Mar 1, 2015 at 3:11 PM, Vladimír Vondruš notifications@github.com
wrote:

Hi,

sorry for the late reply, I somehow completely missed this ...

For the PIC error, my original assumption was that one would always link
the *Context and *Application static libraries to the final executable
and not to any "intermediate" dynamic lib, in which case the
Position-Independent-Code flag wouldn't be needed. I'll fix that shortly.

For the double deletion crash, do you have any small repro code which I
could use for investigating the issue?

Thanks.


Reply to this email directly or view it on GitHub
#90 (comment).

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 1, 2015

Owner

PIC should be fixed in ce82c7f.

Yeah, you are right, Corrade::Utility::String has static std::string members. Can you try commenting them out?

Owner

mosra commented Mar 1, 2015

PIC should be fixed in ce82c7f.

Yeah, you are right, Corrade::Utility::String has static std::string members. Can you try commenting them out?

@Bazilikum

This comment has been minimized.

Show comment
Hide comment
@Bazilikum

Bazilikum Mar 1, 2015

Yepp, i changed the static members Whitespace and Bom to static functions
whitespace() and bom() and changed relevant cpp's (configuration.cpp) to
match change and it seems to fix the error :)
I would suggest making the change then to Corrade.
From what I found out reading about these kind of problems with static
globals I would also suggest a change to DefaultFrameBuffer wrapping it in
a function:
DefaultFramebuffer& defaultFramebuffer(){
static DefaultFramebuffer _dfb;
return _dfb;
}
I am totally unsure if it actually makes any difference but I found that to
be recommended in these kind of cases.
What do you think about this?

Other than that thanks for this great library and your help :)
Will let you know if the problem comes back.

Cheers,
Amit.

On Sun, Mar 1, 2015 at 4:03 PM, Vladimír Vondruš notifications@github.com
wrote:

PIC should be fixed in ce82c7f
ce82c7f
.

Yeah, you are right, Corrade::Utility::String has static std::string
members
https://github.com/mosra/corrade/blob/master/src/Corrade/Utility/String.h#L49-L62.
Can you try commenting them out?


Reply to this email directly or view it on GitHub
#90 (comment).

Bazilikum commented Mar 1, 2015

Yepp, i changed the static members Whitespace and Bom to static functions
whitespace() and bom() and changed relevant cpp's (configuration.cpp) to
match change and it seems to fix the error :)
I would suggest making the change then to Corrade.
From what I found out reading about these kind of problems with static
globals I would also suggest a change to DefaultFrameBuffer wrapping it in
a function:
DefaultFramebuffer& defaultFramebuffer(){
static DefaultFramebuffer _dfb;
return _dfb;
}
I am totally unsure if it actually makes any difference but I found that to
be recommended in these kind of cases.
What do you think about this?

Other than that thanks for this great library and your help :)
Will let you know if the problem comes back.

Cheers,
Amit.

On Sun, Mar 1, 2015 at 4:03 PM, Vladimír Vondruš notifications@github.com
wrote:

PIC should be fixed in ce82c7f
ce82c7f
.

Yeah, you are right, Corrade::Utility::String has static std::string
members
https://github.com/mosra/corrade/blob/master/src/Corrade/Utility/String.h#L49-L62.
Can you try commenting them out?


Reply to this email directly or view it on GitHub
#90 (comment).

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 1, 2015

Owner

The Corrade issue should be fixed in mosra/corrade@266e2d6.

I don't like the idea with having defaultFramebuffer as a function, there must be some other way, similarly to what's done with std::cout etc. I'll try to find some other solution.

Thanks for the thanks, by the way :)

Owner

mosra commented Mar 1, 2015

The Corrade issue should be fixed in mosra/corrade@266e2d6.

I don't like the idea with having defaultFramebuffer as a function, there must be some other way, similarly to what's done with std::cout etc. I'll try to find some other solution.

Thanks for the thanks, by the way :)

@Bazilikum

This comment has been minimized.

Show comment
Hide comment
@Bazilikum

Bazilikum Mar 2, 2015

cool, I hope I helped a bit :)

Im interested, why wouldnt you want it as a function? out of curiosity.
I think iostream and stuff like that are always linked dynamically or?

anyway thanks again :)
Amit.

On Sun, Mar 1, 2015 at 8:22 PM, Vladimír Vondruš notifications@github.com
wrote:

The Corrade issue should be fixed in mosra/corrade@266e2d6
mosra/corrade@266e2d6.

I don't like the idea with having defaultFramebuffer as a function, there
must be some other way, similarly to what's done with std::cout etc. I'll
try to find some other solution.

Thanks for the thanks, by the way :)


Reply to this email directly or view it on GitHub
#90 (comment).

Bazilikum commented Mar 2, 2015

cool, I hope I helped a bit :)

Im interested, why wouldnt you want it as a function? out of curiosity.
I think iostream and stuff like that are always linked dynamically or?

anyway thanks again :)
Amit.

On Sun, Mar 1, 2015 at 8:22 PM, Vladimír Vondruš notifications@github.com
wrote:

The Corrade issue should be fixed in mosra/corrade@266e2d6
mosra/corrade@266e2d6.

I don't like the idea with having defaultFramebuffer as a function, there
must be some other way, similarly to what's done with std::cout etc. I'll
try to find some other solution.

Thanks for the thanks, by the way :)


Reply to this email directly or view it on GitHub
#90 (comment).

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 2, 2015

Owner

Because it's counterintuitive. Consider this (simplified for clarity):

Texture2D blurTexture;
Framebuffer blurFramebuffer(defaultFramebuffer.viewport());
blurFramebuffer.attachTexture(Framebuffer::ColorAttachment{0}, blurTexture, 0)
               .clear(FramebufferClear::Color|FramebufferClear::Depth)
               .bind(FramebufferTarget::Draw);

// draw the scene to blurFramebuffer

defaultFramebuffer.clear(FramebufferClear::Color)
                  .bind(FramebufferTarget::Draw);

// blur contents of blurTexture into defaultFramebuffer

The defaultFramebuffer can be seen as just another framebuffer instance and so you don't have to think differently when using it, it's nearly the same API. Having it as function breaks the consistence.

Also, I have some doubts whether the function-local static variable would really fix the issue -- I once ran into a problem where the static instance ended up being duplicated and different portions of the code used different instance, causing strange issues (but no memory corruption, amazingly enough). I have a feeling that with the function we still have two independent instances, they just aren't causing memory corruption on exit.

I think that, currently, after getting rid of the ugly static std::string instances (which were a bad idea anyway), the remaining issue is the linking order. If I understood correctly, you have this:

b.so
  -> magnum.a

a.so
  -> magnum.a
  -> b.so

app
  -> a.so
  -> b.so

The problem, I think, is that you first link magnum.a into a.so, which will copy the defaultFramebuffer instance into a.so. Then you link b.so into a.so, which already has another copy of defaultFramebuffer instance, thus you end up with two instances (one copied in, one just linked in), but somehow both destructors would be fired on only one of them. Reversing the link order should fix that:

a.so
  -> b.so
  -> magnum.a

This way the defaultFramebuffer instance shouldn't be copied into a.so from magnum.a, as there is already the one linked in from b.so.

If I misunderstood the problem or this doesn't work for you then I'm sorry and I need to find another solution :)

Owner

mosra commented Mar 2, 2015

Because it's counterintuitive. Consider this (simplified for clarity):

Texture2D blurTexture;
Framebuffer blurFramebuffer(defaultFramebuffer.viewport());
blurFramebuffer.attachTexture(Framebuffer::ColorAttachment{0}, blurTexture, 0)
               .clear(FramebufferClear::Color|FramebufferClear::Depth)
               .bind(FramebufferTarget::Draw);

// draw the scene to blurFramebuffer

defaultFramebuffer.clear(FramebufferClear::Color)
                  .bind(FramebufferTarget::Draw);

// blur contents of blurTexture into defaultFramebuffer

The defaultFramebuffer can be seen as just another framebuffer instance and so you don't have to think differently when using it, it's nearly the same API. Having it as function breaks the consistence.

Also, I have some doubts whether the function-local static variable would really fix the issue -- I once ran into a problem where the static instance ended up being duplicated and different portions of the code used different instance, causing strange issues (but no memory corruption, amazingly enough). I have a feeling that with the function we still have two independent instances, they just aren't causing memory corruption on exit.

I think that, currently, after getting rid of the ugly static std::string instances (which were a bad idea anyway), the remaining issue is the linking order. If I understood correctly, you have this:

b.so
  -> magnum.a

a.so
  -> magnum.a
  -> b.so

app
  -> a.so
  -> b.so

The problem, I think, is that you first link magnum.a into a.so, which will copy the defaultFramebuffer instance into a.so. Then you link b.so into a.so, which already has another copy of defaultFramebuffer instance, thus you end up with two instances (one copied in, one just linked in), but somehow both destructors would be fired on only one of them. Reversing the link order should fix that:

a.so
  -> b.so
  -> magnum.a

This way the defaultFramebuffer instance shouldn't be copied into a.so from magnum.a, as there is already the one linked in from b.so.

If I misunderstood the problem or this doesn't work for you then I'm sorry and I need to find another solution :)

@Bazilikum

This comment has been minimized.

Show comment
Hide comment
@Bazilikum

Bazilikum Mar 2, 2015

I updated my local Magnum and Corrade to the latest fixes, and kept the
DefaultFramebuffer like you made it, and have no problems, so I guess the
problem was only with the strings.
If I understand you correctly, then I think you misunderstood me :)
I only linked Magnum once to 'a', and linked 'b' to 'a'.
so its
b.so
->a.so
a.so
->magnum.a
app
->a.so
->b.so
Is that what you meant I should be doing?

So to sum it up, it seems to be working now, and I get no errors. :)

On Mon, Mar 2, 2015 at 1:51 PM, Vladimír Vondruš notifications@github.com
wrote:

Because it's counterintuitive. Consider this (simplified for clarity):

Texture2D blurTexture;
Framebuffer blurFramebuffer(defaultFramebuffer.viewport());
blurFramebuffer.attachTexture(Framebuffer::ColorAttachment{0}, blurTexture, 0)
.clear(FramebufferClear::Color|FramebufferClear::Depth)
.bind(FramebufferTarget::Draw);
// draw the scene to blurFramebuffer

defaultFramebuffer.clear(FramebufferClear::Color)
.bind(FramebufferTarget::Draw);
// blur contents of blurTexture into defaultFramebuffer

The defaultFramebuffer can be seen as just another framebuffer instance
and so you don't have to think differently when using it, it's nearly the
same API. Having it as function breaks the consistence.

Also, I have some doubts whether the function-local static variable would
really fix the issue -- I once ran into a problem where the static instance
ended up being duplicated and different portions of the code used different
instance, causing strange issues (but no memory corruption, amazingly
enough). I have a feeling that with the function we still have two
independent instances, they just aren't causing memory corruption on exit.

I think that, currently, after getting rid of the ugly static std::string
instances (which were a bad idea anyway), the remaining issue is the
linking order. If I understood correctly, you have this:

b.so
-> magnum.a

a.so
-> magnum.a
-> b.so

app
-> a.so
-> b.so

The problem, I think, is that you first link magnum.a into a.so, which
will copy the defaultFramebuffer instance into a.so. Then you link b.so
into a.so, which already has another copy of defaultFramebuffer instance,
thus you end up with two instances (one copied in, one just linked in), but
somehow both destructors would be fired on only one of them. Reversing the
link order should fix that:

a.so
-> b.so
-> magnum.a

This way the defaultFramebuffer instance shouldn't be copied into a.so
from magnum.a, as there is already the one linked in from b.so.

If I misunderstood the problem or this doesn't work for you then I'm sorry
and I need to find another solution :)


Reply to this email directly or view it on GitHub
#90 (comment).

Bazilikum commented Mar 2, 2015

I updated my local Magnum and Corrade to the latest fixes, and kept the
DefaultFramebuffer like you made it, and have no problems, so I guess the
problem was only with the strings.
If I understand you correctly, then I think you misunderstood me :)
I only linked Magnum once to 'a', and linked 'b' to 'a'.
so its
b.so
->a.so
a.so
->magnum.a
app
->a.so
->b.so
Is that what you meant I should be doing?

So to sum it up, it seems to be working now, and I get no errors. :)

On Mon, Mar 2, 2015 at 1:51 PM, Vladimír Vondruš notifications@github.com
wrote:

Because it's counterintuitive. Consider this (simplified for clarity):

Texture2D blurTexture;
Framebuffer blurFramebuffer(defaultFramebuffer.viewport());
blurFramebuffer.attachTexture(Framebuffer::ColorAttachment{0}, blurTexture, 0)
.clear(FramebufferClear::Color|FramebufferClear::Depth)
.bind(FramebufferTarget::Draw);
// draw the scene to blurFramebuffer

defaultFramebuffer.clear(FramebufferClear::Color)
.bind(FramebufferTarget::Draw);
// blur contents of blurTexture into defaultFramebuffer

The defaultFramebuffer can be seen as just another framebuffer instance
and so you don't have to think differently when using it, it's nearly the
same API. Having it as function breaks the consistence.

Also, I have some doubts whether the function-local static variable would
really fix the issue -- I once ran into a problem where the static instance
ended up being duplicated and different portions of the code used different
instance, causing strange issues (but no memory corruption, amazingly
enough). I have a feeling that with the function we still have two
independent instances, they just aren't causing memory corruption on exit.

I think that, currently, after getting rid of the ugly static std::string
instances (which were a bad idea anyway), the remaining issue is the
linking order. If I understood correctly, you have this:

b.so
-> magnum.a

a.so
-> magnum.a
-> b.so

app
-> a.so
-> b.so

The problem, I think, is that you first link magnum.a into a.so, which
will copy the defaultFramebuffer instance into a.so. Then you link b.so
into a.so, which already has another copy of defaultFramebuffer instance,
thus you end up with two instances (one copied in, one just linked in), but
somehow both destructors would be fired on only one of them. Reversing the
link order should fix that:

a.so
-> b.so
-> magnum.a

This way the defaultFramebuffer instance shouldn't be copied into a.so
from magnum.a, as there is already the one linked in from b.so.

If I misunderstood the problem or this doesn't work for you then I'm sorry
and I need to find another solution :)


Reply to this email directly or view it on GitHub
#90 (comment).

@mosra

This comment has been minimized.

Show comment
Hide comment
@mosra

mosra Mar 2, 2015

Owner

Yeah, I got that wrong, heh. The only problem would be when b.so uses symbols from magnum.a that aren't used in a.so, in which case you would need to link magnum.a into b.so again, but after a.so, to avoid having the static symbols duplicated.

Great, glad it's solved without severe architectural changes :)

Owner

mosra commented Mar 2, 2015

Yeah, I got that wrong, heh. The only problem would be when b.so uses symbols from magnum.a that aren't used in a.so, in which case you would need to link magnum.a into b.so again, but after a.so, to avoid having the static symbols duplicated.

Great, glad it's solved without severe architectural changes :)

@mosra mosra closed this Mar 2, 2015

@mosra mosra added this to the 2015.05 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment