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

Allow a fiber in HOLD state to be reset bypassing scope(exit) etc. #1252

Merged
merged 3 commits into from May 7, 2015

Conversation

Dzugaru
Copy link
Contributor

@Dzugaru Dzugaru commented May 4, 2015

* so it may leak resources which will lead to stack overflow if reset
* is called too many times for such a fiber. It will
* properly reset the stack though, so the fiber should behave like
* a new one.
Copy link
Member

Choose a reason for hiding this comment

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

That's actually much more confusing than before.

One should only reset a fiber in term state, because resetting it in hold state won't cleanup the stack, e.g. scope(exit) and struct destructors aren't called.

@MartinNowak
Copy link
Member

Looking at the implementation again, there is indeed a bug.
The assert( m_ctxt.tstack == m_ctxt.bstack ); in reset will only work for terminated fibers not for hold ones. We'd need to explicitly set m_ctxt.tstack = m_ctxt.bstack for those.

@Dzugaru
Copy link
Contributor Author

Dzugaru commented May 4, 2015

Now I get it, reset without parameters works for HOLD state iff the stack is clean. Reset with parameters (new function/delegate) works with any state cause there is no check there and leads to stack overflow.

We'd need to explicitly set m_ctxt.tstack = m_ctxt.bstack for those.

That would be awesome, one could cope with scope(exit) and struct destructors not executing but having a way to completely reset HOLD fiber is very nice.

@MartinNowak
Copy link
Member

We'd need to explicitly set m_ctxt.tstack = m_ctxt.bstack for those.

Can you change the pull to just do that and add a test case.

unittest
{
    // test unsafe reset in hold state
    auto fib = new Fiber({ubyte[2048] buf = void; Fiber.yield();}, 4096);
    foreach (_; 0 .. 10)
    {
        fib.call();
        assert(fib.state == Fiber.State.HOLD);
        fib.reset();
    }
}

@Dzugaru
Copy link
Contributor Author

Dzugaru commented May 6, 2015

Can you change the pull to just do that and add a test case.

Did that, rebuilt everything with Digger on win32 and that test case works.

@Dzugaru Dzugaru changed the title Update thread.d Fiber documentation Allow a fiber in HOLD state to be reset bypassing scope(exit) etc. May 6, 2015
@MartinNowak
Copy link
Member

Auto-merge toggled on

@MartinNowak
Copy link
Member

Thx

MartinNowak added a commit that referenced this pull request May 7, 2015
Allow a fiber in HOLD state to be reset bypassing scope(exit) etc.
@MartinNowak MartinNowak merged commit 2c74db0 into dlang:master May 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants