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

Conversation

dnadlinger
Copy link
Contributor

@dnadlinger
Copy link
Contributor Author

Ping @alexrp, @MartinNowak. This is quite a severe issue for any project using fibers (such as the Weka code base).

dnadlinger added a commit to ldc-developers/druntime that referenced this pull request Sep 24, 2015
@ibuclaw
Copy link
Member

ibuclaw commented Sep 24, 2015

👍 - Fix works for gdc too.

@MartinNowak
Copy link
Member

Can we store it as Fiber field and on the stack instead? I want to get rid of the Context within chain at some point.

@dnadlinger
Copy link
Contributor Author

Context seems like the most natural place right now, and this does not add any extra work for getting rid of it – just do the same thing to the EH context pointer that you are going to do to the stack top/bottom pointers.

@MartinNowak
Copy link
Member

I want to keep the array of Contexts with stacktop/bottom for the GC, the nested push/pop Context is only needed to find out whether the stack pointer during a GC suspend is in the thread's stack or the current Fiber's stack. This can be done by checking the stack range instead.
But anyhow, keeping this variable on the stack is simple enough to do later.

MartinNowak added a commit that referenced this pull request Sep 25, 2015
Fix Issue 15104 - Fiber context switch in finally blocks breaks EH
@MartinNowak MartinNowak merged commit f0f8eda into dlang:master Sep 25, 2015
@dnadlinger dnadlinger deleted the fix-15104 branch September 25, 2015 12:33
@dnadlinger
Copy link
Contributor Author

Hm, yeah, I thought I'd need to modify the asm code for this to push the extra value (which would have been annoying due to alignment, different platforms, etc.), but we could probably just save it on the stack in the context switching member functions.

@MartinNowak
Copy link
Member

I think you can because the context switching itself does not throw.

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.

3 participants