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

fix issue 18547 - Win32: throwing exception in fiber crashes application #2129

Merged
merged 1 commit into from Mar 11, 2018

Conversation

rainers
Copy link
Member

@rainers rainers commented Mar 4, 2018

increase the default stack size because exception handling might need up to 16k. The actually used stack can depend on the version of DbgHelp.dll, the existence of debug information and possibly other conditions.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @rainers!

Bugzilla references

Auto-close Bugzilla Severity Description
18547 normal Win32: throwing exception in fiber crashes application

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Mar 4, 2018
@@ -4087,7 +4086,7 @@ class Fiber
* In:
* fn must not be null.
*/
this( void function() fn, size_t sz = PAGESIZE*4,
this( void function() fn, size_t sz = size_t.max,
Copy link
Member

Choose a reason for hiding this comment

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

Could we remove the stack size as the default value for this parameter, and add set of the constructors this (void function()/*delegate*/ fn) which will default to 8 pages or 4 pages, depending on platform. If you keep the existing approach, perhaps the documentation of these constructors should indicate default stack size?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be possible, but would duplicate the number of necessary ctors and the documentation.
I was also wondering whether the default 4/8*PAGESIZE makes much sense if the system uses large pages. (I started using enum defaultStackSize just to notice that PAGESIZE is runtime-initialized).

Copy link
Member Author

Choose a reason for hiding this comment

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

On second thought, maybe it is better to change the default-argument to defaultStackPages*PAGESIZE, as it is more self-documenting and does not need a magic value.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is better to change the default-argument to defaultStackPages*PAGESIZE

Done.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, tnx

increase the default stack size because exception handling might need up to 16k. The actually used stack can depend on the version of DbgHelp.dll, the existence of debug information and possibly other conditions.
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
5 participants