Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Conversation

@wilzbach
Copy link
Contributor

See also: #2104 (comment)

@wilzbach wilzbach requested a review from andralex as a code owner February 23, 2018 19:15
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@schveiguy
Copy link
Member

schveiguy commented Feb 23, 2018

I think there is a different problem here!

This does not segfault:

import core.memory;
void main()
{
   int *x = null;
   GC.free(x);
}

x = null;
if (x !is null)
{
GC.free(&x);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong, shouldn't it be GC.free(x) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like I was a bit too fast. Thanks!

@schveiguy
Copy link
Member

OK, I see you fixed that problem. But now I'm slightly confused!

Because extracting out the problem:

void main()
{
    int *ptr = null;
    GC.free(&ptr);
}

This segfaults, but this:

void main()
{
   int *ptr = new int(1);
   GC.free(&ptr);
}

DOESN'T segfault. Neither is a valid heap pointer, so why does it segfault at all? The first thing GC.free should do is to find the pool the pointer is in, and it should fail in both cases.

In any case, a bug for another day. Your changes LGTM.

@schveiguy
Copy link
Member

Hah, I think I know why it was segfaulting. Because of the new ProxyGC! Attempting to free a pointer before actually allocating any pointer will do this: https://github.com/dlang/druntime/blob/master/src/gc/impl/proto/gc.d#L140, which equates to a segfault when compiled in release mode.

@schveiguy
Copy link
Member

This does fix a bug, though (__delete was not actually freeing the memory), so I'm pulling.

@dlang-bot dlang-bot merged commit f496294 into dlang:stable Feb 24, 2018
@wilzbach wilzbach deleted the __delete-segfault branch February 24, 2018 11:04
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.

4 participants