search

Abscissa / libInputVisitor

Write D input range generators in a straightforward coroutine style
3 stars 0 forks source link

Call to Fiber crashes an application. [Windows x86_64] #1

Closed MrSmith33 closed 8 years ago

MrSmith33 commented 8 years ago

When using sdlang-d 0.9.5 I found that call() inside InputVisitor fails unless I give more memory to the Fiber. This happens only in 64 bit mode. Here is my test case: https://gist.github.com/MrSmith33/c2f435937aefbc3eead25aa873363d84

Abscissa commented 8 years ago

Which version of the compiler are you using? And which version of dub?

MrSmith33 commented 8 years ago

DMD32 D Compiler v2.071.1-b1 DUB version 1.0.0

Abscissa commented 8 years ago

I'm unable to reproduce. Using:

C:\DevProj\test\libInputVisitor-issue1>dub --help | grep "DUB version"
DUB version 1.0.0, built on Jun 20 2016

C:\DevProj\test\libInputVisitor-issue1>dmd | grep DMD
DMD32 D Compiler v2.071.1-b1

C:\DevProj\test\libInputVisitor-issue1>dir
 Volume in drive C is OS
 Volume Serial Number is 008E-DBB0

 Directory of C:\DevProj\test\libInputVisitor-issue1

09/19/2016  11:15 AM    <DIR>          .
09/19/2016  11:15 AM    <DIR>          ..
09/19/2016  10:52 AM               558 client.sdl
09/19/2016  11:08 AM               131 dub.json
09/19/2016  11:08 AM                57 main.d
               3 File(s)            746 bytes
               2 Dir(s)  23,631,859,712 bytes free

C:\DevProj\test\libInputVisitor-issue1>dub --arch=x86_64 --compiler=dmd
Package sdlang-d 0.8.3 could not be loaded either locally, or from the configured package registries.
Package sdlang-d 0.8.2 could not be loaded either locally, or from the configured package registries.
Package sdlang-d 0.8.3 could not be loaded either locally, or from the configured package registries.
Package sdlang-d 0.8.2 could not be loaded either locally, or from the configured package registries.
Performing "debug" build using dmd for x86_64.
libinputvisitor 1.2.0: target for configuration "library" is up to date.
sdlang-d 0.9.5: target for configuration "library" is up to date.
test ~master: building configuration "application"...
..\..\..\Users\Nick\AppData\Roaming\dub\packages\sdlang-d-0.9.5\sdlang-d\src\sdlang\lexer.d(13,8): Deprecation: module std.stream is deprecated - It will be removed from Phobos in October 2016. If you still need it, go to https://github.com/DigitalMars/undeaD
Linking...
To force a rebuild of up-to-date targets, run again with --force.
Running .\test.exe

C:\DevProj\test\libInputVisitor-issue1>

Can you show me the exact change you made to make it work for you? Maybe submit a PR?

Abscissa commented 8 years ago

Oh, I can reproduce using --build=release. Debug build doesn't seem to exhibit the problem for me. (Wonder if it's a DMD release-mode bug?) But again, post the change you made that made it work for you and I'll verify & merge.

MrSmith33 commented 8 years ago

I replaced this https://github.com/Abscissa/libInputVisitor/blob/master/libInputVisitor.d#L41 with this: super(&run, 4096 * 16); Which increases stack size from 4096 to 4096 * 16. I've checked right now, and it works begining from 4069 * 3 and fails at 4096 * 2.

MrSmith33 commented 8 years ago

But that can also brake if stack happens to be bigger that the one used in sdlang. So using fiber here is a bad idea imho.

Abscissa commented 8 years ago

Yea, honestly, I don't like using fibers for this sort of thing either, but AFAIK its really the only way to do a coroutine-style function in D without loosing the ability to provide a range interface. And writing data generators as input ranges, especially for parsers, tends makes a colossal mess of otherwise-straightforward code (on par with the callback-frenzy in Node.js or in Flash's ActionScript2 if anyone remembers that). It just turns the whole algorithm you're writing completely inside-out. Much as I prefer D to C#, I really wish it had C#'s stackless coroutines. C even has one (forget the name of the lib) that makes use of the preprocessor, but it wouldn't work so well in D.

Back when ranges were still kinda new, I tried to push the whole idea of writing input ranges in a C#-like stackless coroutine manner, but nobody really was interested.

But I could rant on that for sooo long...

I'll see what I can do about dealing with the stack size (although it looks like the default is PAGESIZE*4 and apparently PAGESIZE is private within druntime??? Ugh.) but something else about this is really bothering me:

Why would we be hitting a stack overflow only in 64-bit and only in release mode. I could understand if it were the the other way around, but this seems very backwards to me. I'll see what I can do to work around it, but I wonder if we're hitting a compiler code-generation bug. And those are NOT fun. :(

MrSmith33 commented 8 years ago

Well, it would be cool if there where some kind of trait that returns stack size of a function. Also, on my machine both debug and release versions are behaving the same.

Abscissa commented 8 years ago

Why would we be hitting a stack overflow only in 64-bit and only in release mode. I could understand if it were the the other way around, but this seems very backwards to me. I'll see what I can do to work around it, but I wonder if we're hitting a compiler code-generation bug. And those are NOT fun. :(

I've filed this: https://issues.dlang.org/show_bug.cgi?id=16511

Abscissa commented 8 years ago

I've worked around the problem by doing two things:

Tagged v1.2.1.

In case there's some other underlying issue as I suspected, I've filed: https://issues.dlang.org/show_bug.cgi?id=16511

A new version of SDLang-D using this will be coming shortly. SDLang-D master is already updated.

So I think this can be closed now. If any problems still occur, either reopen this or file a new issue.