Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call stack size exceeded of wasm asyncify #87

Closed
quolpr opened this issue May 3, 2023 · 7 comments
Closed

Call stack size exceeded of wasm asyncify #87

quolpr opened this issue May 3, 2023 · 7 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@quolpr
Copy link

quolpr commented May 3, 2023

Hey! I found one weird bug, I have RangeErrors for some queries when I use asyncified version of wa-sqlite. SQLite just fails to prepare this query:

VALUES(0) UNION VALUES(1) UNION VALUES(2) UNION VALUES(3) UNION VALUES(4) UNION VALUES(5) UNION VALUES(6) UNION VALUES(7) UNION VALUES(8) UNION VALUES(9) UNION VALUES(10) 
UNION VALUES(11) UNION VALUES(12) UNION VALUES(13) UNION VALUES(14) UNION VALUES(15) UNION VALUES(16) UNION VALUES(17) UNION VALUES(18) UNION VALUES(19) UNION VALUES(20) 
UNION VALUES(21) UNION VALUES(22) UNION VALUES(23) UNION VALUES(24) UNION VALUES(25) UNION VALUES(26) UNION VALUES(27) UNION VALUES(28) UNION VALUES(29) UNION VALUES(30) 
UNION VALUES(31) UNION VALUES(32) UNION VALUES(33) UNION VALUES(34) UNION VALUES(35) UNION VALUES(36) UNION VALUES(37) UNION VALUES(38) UNION VALUES(39) UNION VALUES(40) 
UNION VALUES(41) UNION VALUES(42) UNION VALUES(43) UNION VALUES(44) UNION VALUES(45) UNION VALUES(46) UNION VALUES(47) UNION VALUES(48) UNION VALUES(49) UNION VALUES(50) 
UNION VALUES(51) UNION VALUES(52) UNION VALUES(53) UNION VALUES(54) UNION VALUES(55) UNION VALUES(56) UNION VALUES(57) UNION VALUES(58) UNION VALUES(59) UNION VALUES(60) 
UNION VALUES(61) UNION VALUES(62) UNION VALUES(63) UNION VALUES(64) UNION VALUES(65) UNION VALUES(66) UNION VALUES(67) UNION VALUES(68) UNION VALUES(69) UNION VALUES(70) 
UNION VALUES(71) UNION VALUES(72) UNION VALUES(73) UNION VALUES(74) UNION VALUES(75) UNION VALUES(76) UNION VALUES(77) UNION VALUES(78) UNION VALUES(79) UNION VALUES(80) 
UNION VALUES(81) UNION VALUES(82) UNION VALUES(83) UNION VALUES(84) UNION VALUES(85) UNION VALUES(86) UNION VALUES(87) UNION VALUES(88) UNION VALUES(89) UNION VALUES(90) 
UNION VALUES(91) UNION VALUES(92) UNION VALUES(93) UNION VALUES(94) UNION VALUES(95) UNION VALUES(96) UNION VALUES(97) UNION VALUES(98) UNION VALUES(99) UNION VALUES(100) 
UNION VALUES(101) UNION VALUES(102) UNION VALUES(103) UNION VALUES(104) UNION VALUES(105) UNION VALUES(106) UNION VALUES(107) UNION VALUES(108) UNION VALUES(109) UNION VALUES(110) 
UNION VALUES(111) UNION VALUES(112) UNION VALUES(113) UNION VALUES(114) UNION VALUES(115) UNION VALUES(116) UNION VALUES(117) UNION VALUES(118) UNION VALUES(119) UNION VALUES(120) 
UNION VALUES(121) UNION VALUES(122) UNION VALUES(123) UNION VALUES(124) UNION VALUES(125) UNION VALUES(126) UNION VALUES(127) UNION VALUES(128) UNION VALUES(129) UNION VALUES(130) 
UNION VALUES(131) UNION VALUES(132) UNION VALUES(133) UNION VALUES(134) UNION VALUES(135) UNION VALUES(136) UNION VALUES(137) UNION VALUES(138) UNION VALUES(139) UNION VALUES(140) 
UNION VALUES(141) UNION VALUES(142) UNION VALUES(143) UNION VALUES(144) UNION VALUES(145) UNION VALUES(146) UNION VALUES(147) UNION VALUES(148) UNION VALUES(149) UNION VALUES(150) 
UNION VALUES(151) UNION VALUES(152) UNION VALUES(153) UNION VALUES(154) UNION VALUES(155) UNION VALUES(156) UNION VALUES(157) UNION VALUES(158) UNION VALUES(159) UNION VALUES(160) 
UNION VALUES(161) UNION VALUES(162) UNION VALUES(163) UNION VALUES(164) UNION VALUES(165) UNION VALUES(166) UNION VALUES(167) UNION VALUES(168) UNION VALUES(169) UNION VALUES(170) 
UNION VALUES(171) UNION VALUES(172) UNION VALUES(173) UNION VALUES(174) UNION VALUES(175) UNION VALUES(176) UNION VALUES(177) UNION VALUES(178) UNION VALUES(179) UNION VALUES(180) 
UNION VALUES(181) UNION VALUES(182) UNION VALUES(183) UNION VALUES(184) UNION VALUES(185) UNION VALUES(186) UNION VALUES(187) UNION VALUES(188) UNION VALUES(189) UNION VALUES(190) 
UNION VALUES(191) UNION VALUES(192) UNION VALUES(193) UNION VALUES(194) UNION VALUES(195) UNION VALUES(196) UNION VALUES(197) UNION VALUES(198)

image

The interesting thing is that:

  1. It runs well when DevTools is opened, and keeps working well after devtool is closed (after closing current tab & opening new it becomes reproducible again)
  2. It runs well on the first attempt. After it fails

I tested it on m1 MacBook with Chrome

112.0.5615.137 (Official Build) (arm64)

The query run ok with any wasm standard building under any condition.

@quolpr quolpr changed the title Asyncified version — RangeError: Maximum call stack size exceeded Call stack size exceeded of wasm asyncify May 3, 2023
@rhashimoto
Copy link
Owner

Can you try:

Then rebuild the WASM artifacts. You would need to follow the build instructions on the README. If you don't have the time or the background, I'll get around to it at some point.

@rhashimoto rhashimoto added the bug Something isn't working label May 4, 2023
@quolpr
Copy link
Author

quolpr commented May 5, 2023

I tried here quolpr@7c0d9bd , but it didn't help. I suspect that this error is about JS VM stack error 🤔

@quolpr
Copy link
Author

quolpr commented May 5, 2023

I also made the debug build, and it seems just fail to parse the query:

image

image

The bad things about this are that for iOS Safari it seems the stack size is not so big and sometime prepare fails even for simple query like CREATE TABLE ....

And yeah, the weird thing is that the first run doesn't fail 🤔

@rhashimoto
Copy link
Owner

Thanks for taking a look! I'm puzzled because I really thought one of those changes would fix it. I'll investigate myself when I can.

This isn't a valid SQL(ite) statement, right? Not that it should produce a stack overflow, but I'm wondering if that is part of what triggers the issue.

@quolpr
Copy link
Author

quolpr commented May 5, 2023

This isn't a valid SQL(ite) statement, right? Not that it should produce a stack overflow, but I'm wondering if that is part of what triggers the issue.

Nope, it's valid. You can run it with not asyncify build, or you can also paste it to here https://sql.js.org/examples/GUI/ . For me, it even fails with 1...100 range (in the example I used 1...200 VALUES).

I used VALUES in one of my queries and now instead I use CTE like here https://til.simonwillison.net/sqlite/cte-values

@rhashimoto
Copy link
Owner

Thanks, I never knew you could use a bare VALUES clause as a statement. Is there a reason you need so many VALUES, as opposed to just:

VALUES (0), (1), (2), ...

The internal functions sqlite3Select() and multiSelect() are mutually recursive, as your debug build stack trace shows, and every VALUES starts a new subquery and introduces another level of recursion.

I think that explains why you're putting so much pressure on the stack, though it doesn't explain why it won't fail on the first try or with dev tools open.

@rhashimoto
Copy link
Owner

I'm going to mark this WONTFIX. I don't see a compelling reason to use nested VALUES in this way causing the deep recursion here. And unfortunately, even if there is a good reason to do this, the "obvious" ways that Emscripten provides to tune the stack size don't seem to have any effect.

@rhashimoto rhashimoto added the wontfix This will not be worked on label May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants