Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

[dmd-cxx] Fix Issue 19481: mutex.d(95) Error: pthread_mutex_init failed #2534

Merged
merged 1 commit into from
Apr 1, 2019
Merged

[dmd-cxx] Fix Issue 19481: mutex.d(95) Error: pthread_mutex_init failed #2534

merged 1 commit into from
Apr 1, 2019

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Mar 31, 2019

Committing the backport before it's in master... should give you a couple months to fix failing mecca in #2411 before gdc switches to ddmd/master when stage1 opens for gcc-10.

@ibuclaw ibuclaw added the Bug Fix Include reference to corresponding bugzilla issue label Mar 31, 2019
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
19481 major Aborting from local/libphobos/libdruntime/core/sync/mutex.d(95) Error: pthread_mutex_init failed.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "dmd-cxx + druntime#2534"

Copy link
Member

@rainers rainers left a comment

Choose a reason for hiding this comment

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

Maybe mutexAlign could be a property of Mutex itself so explicit alignment could be used by overwriting mutexAlign with a constant.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 1, 2019

There's nothing wrong with the alignment of Mutex. If you're lucky, it's the size of N in void[N][2] that's the issue, as only the first index is suitably aligned. (You're unlucky if the alignment of pointers is smaller than the alignment of the OS mutex type).

There is no one constant alignment either, what type of and what alignment of the mutex is varies between platforms.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 1, 2019

Committing, if an amendment is needed before May, then so be it.

@ibuclaw ibuclaw merged commit d57fa1f into dlang:dmd-cxx Apr 1, 2019
@ibuclaw ibuclaw deleted the dmd-cxx-mutexalign branch April 1, 2019 10:43
iains pushed a commit to iains/gcc-git that referenced this pull request Apr 1, 2019
libphobos: Fix abort in pthread_mutex_init on Solaris.

Merges upstream druntime d57fa1ff.

Reviewed-on: dlang/druntime#2534

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@270057 138bc75d-0d04-0410-961f-82ee72b054a4
@rainers
Copy link
Member

rainers commented Apr 1, 2019

There's nothing wrong with the alignment of Mutex.

Mutex.alignof might be unusable here because it is the alignment of the class reference (the unlucky case):

class C
{
	double x;
}

struct S
{
	align(16) int x;
}

// for -m32
pragma(msg, Object.alignof); // 4
pragma(msg, C.alignof);      // 4
pragma(msg, C.x.alignof);    // 8
pragma(msg, S.alignof);      // 16
pragma(msg, S.x.alignof);    // 4

Agreed, the aligned size of the object is another issue.

There is no one constant alignment either, what type of and what alignment of the mutex is varies between platforms.

I meant something like

version(mySpecialOS)
    enum mutexAlign = 16; // due to explicit alignment on field of mutex
else
    enum mutexAlign = classInstanceAlignment!Mutex;

so this declaration doesn't have to be repeated everytime something similar is done.

But probably the better solution would be for alignof(C.x) to return the explicit alignment for symbols, not the alignment of the type.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 1, 2019

Mutex.alignof might be unusable here because it is the alignment of the class reference (the unlucky case):

Yes, you can't use Mutex.alignof, which why it is being changed from. The underlying record is suitably aligned, hence I re-iterate there is no problem with Mutex (i.e: when used as new Mutex()).

I meant something like

I don't see a reason for that, as you almost certainly mean OS_mutex_type.alignof (Currently: CRITICAL_SECTION on Windows, pthread_mutex_t on Posix, etc...) rather than a hard-coded number.

I had considered something similar to core.sync.semaphore.

public:
    /// Aliases the operating-system-specific mutex type.
    version (Windows)        alias Handle = CRITICAL_SECTION;
    /// ditto
    else version (Posix)     alias Handle = pthread_mutex_t;

private:
    /// Handle to the system-specific mutex.
    Handle m_hndl;

But ultimately went for this instead over align(Mutex.Handle.alignof) xxx, as I'd rather not assume that the OS mutex has the greater alignment requirement of the class.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue
Projects
None yet
4 participants