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

Conversation

MartinNowak
Copy link
Member

  • as signals are no longer send during thread startup we can use
    a TLS variable to store Thread.getThis()
  • make sure to set the variable (and probably trigger lazy TLS
    allocation) before adding the Thread to the global list

Issue 15270 – use TLS to store Thread.getThis (pthread_getspecific causes heavy lock contention)

@MartinNowak
Copy link
Member Author

Depends on #1421, please merge that beforehand.

@MartinNowak
Copy link
Member Author

This is a much nicer solution than what we tried before (#1116, #1164), meaning we can now retackle parallel suspend much easier.
https://trello.com/c/lddgsl6f/14-parallel-suspend

@dnadlinger
Copy link
Contributor

Can you rebase, please? Also, this does not pass on Darwin.

@MartinNowak
Copy link
Member Author

Yes, I have to debug why the removal from the aboutToStart array fails.

@dnadlinger
Copy link
Contributor

Let me know if you need help with the OS X part (although I would imagine you have access to a Mac anyway).

@MartinNowak
Copy link
Member Author

Let me know if you need help with the OS X part (although I would imagine you have access to a Mac anyway).

Yes, VirtualBox.

Damn, looks like a race condition.
https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=1813353&isPull=true
https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=1813843&isPull=true

- only keep threads in global thread list while they are running
  this directly avoids any issues with signals delivered during
  thread startup
- add an aboutToStart array to keep track of just spawned threads
  this is needed so that thread_joinAll doesn't miss a thread
  also the windows dll_attach_thread code looks up a just spawned thread
- remove all the misleading comments
- remove all the cargo cult handling of situations that can no longer
  occur, e.g. suspendDepth being set while adding a thread
- as signals are no longer send during thread startup we can use
  a TLS variable to store Thread.getThis()
- make sure to set the variable (and probably trigger lazy TLS
  allocation) before adding the Thread to the global list
- between pthread_create(&m_addr) and reading m_addr to
  asserting isRunning when adding the newly created thread
- this race existed before but didn't manifest b/c the thread was added
  by the starting thread (after pthread_create) returned
- check isRunning after acquiring slock to ensure pthread_create already
  returned
@MartinNowak
Copy link
Member Author

Found it, only took a few hours :(. The stupid pthread_create implementation on OSX doesn't assign m_addr before launching the thread, which tripped a rather superfluous assertion.

@dnadlinger
Copy link
Contributor

Auto-merge toggled on

dnadlinger added a commit that referenced this pull request Oct 31, 2015
fix Issue 15270 - use TLS to store Thread.getThis
@dnadlinger dnadlinger merged commit c2c255c into dlang:master Oct 31, 2015
@MartinNowak MartinNowak deleted the fix15670 branch October 31, 2015 17:39
JohanEngelen added a commit to weka/druntime that referenced this pull request Mar 16, 2016
…-2.070

Conflicts:
	src/core/thread.d
Resolved the conflicts in favor of LDC's druntime. The conflicting changes concerned 'UsePthreadTlsForThreadThis', which are no longer needed: "This becomes obsolete with the a rewrite of the related code in 2.069: // dlang#1422."
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