Skip to content

Fix assertion in Mutex::destroy() on DragonFly (#20698) - #20741

Merged
bors merged 1 commit into
rust-lang:masterfrom
mneumann:dragonfly-pthread-mutex
Jan 9, 2015
Merged

Fix assertion in Mutex::destroy() on DragonFly (#20698)#20741
bors merged 1 commit into
rust-lang:masterfrom
mneumann:dragonfly-pthread-mutex

Conversation

@mneumann

@mneumann mneumann commented Jan 8, 2015

Copy link
Copy Markdown
Contributor

On DragonFly pthread_mutex_destroy() returns EINVAL (22) if called on a
pthread_mutex_t that was just initialized via PTHREAD_MUTEX_INITIALIZER
and not used otherwise or initialized via pthread_mutex_init(). Change
the code to treat a return value of EINVAL on DragonFly as success, too.

This fixes issue #20698.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

Comment thread src/libstd/sys/unix/mutex.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an EINVAL const somewhere in liblibc you could use instead of a raw 22?

@mneumann

mneumann commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

@sfackler: I will look out for EINVAL and try to use it.

There are some more similar patches to come for condvar.rs and rwlock.rs. Cross-compiling to DragonFly is slow and I thought this would fix all my issues...

On DragonFly pthread_{mutex,rwlock,condvar}_destroy() returns EINVAL
when called on a pthread_{mutex,rwlock,condvar}_t that was just
initialized via PTHREAD_{MUTEX,RWLOCK,CONDVAR}_INITIALIZER and not used
in the meantime or initialized via pthread_{mutex,rwlock,condvar}_init().
Change the code to treat a return value of EINVAL on DragonFly as success.
@mneumann

mneumann commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

Ok, now I also fixed condvar and rwlock, now it's building a working Rust compiler for DragonFly!!!

@alexcrichton

Copy link
Copy Markdown
Member

I do think that this may actually be a bug in the pthread implementation, it sounds like patterns like this are supposed to work. We've had problems in the past with buggy pthreads implementations as well.

That being said, I'm not totally opposed to having a special case. It's a little unfortunate to start growing an array of #[cfg] directives that keep growing, but it is fairly isolated.

Out of curiosity, do you know if this issue could be brought up with DragonFly maintainers?

@mneumann

mneumann commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

@alexcrichton: The related DragonFly bug report is here, but I doubt it will change, and even if it will be "fixed", there will always be older versions of DragonFly and Rust won't work on them. The way it is implemented in DragonFly protects you against duplicate pthread_mutex_destroy() calls, which IMHO is a good thing, except that it breaks the pthread spec which is saying that PTHREAD_MUTEX_INITIALIZER should have the same effect as calling pthread_mutex_init(), which is not true on DragonFly. So I'd suggest to live with this #[cfg]-magic, hopefully it'll be the last required to get it working on DragonFly. One way to fix it in DragonFly would be to use a special sentinel value (e.g. a (void*)1 pointer; NULL is used as initializer) which denotes that the mutex was destroyed.

@alexcrichton

Copy link
Copy Markdown
Member

Alright, sounds good to me! Thanks for opening a report with them :)

@mneumann

mneumann commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

@alexcrichton: We fixed it with this commit. It's now questionable if we still need to apply this pull request. I can update my DragonFly system, anyone else who wants to run Rust should do so too.

@alexcrichton

Copy link
Copy Markdown
Member

Nice! Do you know how quickly updates roll out to DragonFly? If it takes awhile, we could just slate this for deletion in 6 months or so.

bors added a commit that referenced this pull request Jan 9, 2015
Fix assertion in Mutex::destroy() on DragonFly (#20698)

Reviewed-by: alexcrichton
@bors
bors merged commit b527494 into rust-lang:master Jan 9, 2015
@mneumann

mneumann commented Jan 9, 2015

Copy link
Copy Markdown
Contributor Author

Usually every 6 months there is a new DragonFly release. Agreed, let's remove it some time in the future. Thanks!

lnicola pushed a commit to lnicola/rust that referenced this pull request Dec 31, 2025
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

Successfully merging this pull request may close these issues.

5 participants