bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[積丹尼 Dan Jacobson]

Instead of warning the user when this happens

KY> (defun shr-descend (dom)
KY>   ...
KY>     (if (> shr-depth (/ max-specpdl-size 15))
KY> 	(setq shr-warning "Too deeply nested to render properly; consider increasing `max-specpdl-size'")
KY>       else ...))

it should ask the user "Should we try 15 levels deeper? y-n

and if that is still not enough,

it should ask the user "Should we try 25 levels even more deeper? y-n
...

Why?

Because the user is facing a probably one-time bad html message.

He doesn't want to change his .emacs to always let them pass.

He also wants the choice:
no I don't want to go deeper for this promotional coupon.
yes my grandmom sent me this so I want to see it.

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Noam Postavsky]

severity 30675 wishlist
quit

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> Instead of warning the user when this happens
>
> KY> (defun shr-descend (dom)
> KY>   ...
> KY>     (if (> shr-depth (/ max-specpdl-size 15))
> KY> 	(setq shr-warning "Too deeply nested to render properly; consider increasing `max-specpdl-size'")
> KY>       else ...))
>
> it should ask the user "Should we try 15 levels deeper? y-n
>
> and if that is still not enough,
>
> it should ask the user "Should we try 25 levels even more deeper? y-n
> ...
>
> Why?
>
> Because the user is facing a probably one-time bad html message.
>
> He doesn't want to change his .emacs to always let them pass.
>
> He also wants the choice:
> no I don't want to go deeper for this promotional coupon.
> yes my grandmom sent me this so I want to see it.

On the other hand, if you say go deeper too far Emacs will crash.  It's
a bit hard to predict how far is too far, so I'm not sure if this prompt
is such a great idea.  See Bug#16999 "calc crashes when computation
limit is increased" for an example of such a prompt.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=16999

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> Instead of warning the user when this happens
>
> KY> (defun shr-descend (dom)
> KY>   ...
> KY>     (if (> shr-depth (/ max-specpdl-size 15))
> KY> 	(setq shr-warning "Too deeply nested to render properly; consider increasing `max-specpdl-size'")
> KY>       else ...))
>
> it should ask the user "Should we try 15 levels deeper? y-n

Or perhaps shr should just bind that variable to, like, 10x what it
normally is?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[積丹尼 Dan Jacobson]

>>>>> "LI" == Lars Ingebrigtsen <larsi@gnus.org> writes:

LI> Or perhaps shr should just bind that variable to, like, 10x what it
LI> normally is?

You're the boss.

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

>>>>>> "LI" == Lars Ingebrigtsen <larsi@gnus.org> writes:
>
> LI> Or perhaps shr should just bind that variable to, like, 10x what it
> LI> normally is?
>
> You're the boss.

No, I'm not.  Eli is.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 13 Apr 2018 02:27:19 +0200
> Cc: 30675@debbugs.gnu.org
> 
> 積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:
> 
> >>>>>> "LI" == Lars Ingebrigtsen <larsi@gnus.org> writes:
> >
> > LI> Or perhaps shr should just bind that variable to, like, 10x what it
> > LI> normally is?
> >
> > You're the boss.
> 
> No, I'm not.  Eli is.  :-)

I am?  Well, if I need to give my opinion on this, then blindly
increasing the limit ten-fold is something that'd make me worry about
a potential C stack overflow.  I'd feel much better with a lower
factor, e.g. some value that is just enough to cover this case plus
some slack.  Bonus points for providing a defcustom with the factor,
so that users could change that.

The idea of asking the user whether to increase by N levels sounds OK
to me, provided that its implementation is not out-worldly hard.

Thanks.

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I'd feel much better with a lower factor, e.g. some value that is just
> enough to cover this case plus some slack.  Bonus points for providing
> a defcustom with the factor, so that users could change that.

The problem is that HTML may nest arbitrarily deep, so there isn't
really any way to cover this completely.  But the larger the stack size
is, the lower the possibility for hitting the limit is.

FWIW, I haven't hit the limit in about a year, so it's rare for people
to encounter HTML that's that deeply nested.

> The idea of asking the user whether to increase by N levels sounds OK
> to me, provided that its implementation is not out-worldly hard.

I kinda feel that stack size is something that a user has little idea of
what it would mean.  Emacs: "We hit a magical limit!  Raise the limit?"
User: "Sure!"  Emacs: "Now it worked!"  I mean...  why consult the user
at all?  :-)

> Well, if I need to give my opinion on this, then blindly
> increasing the limit ten-fold is something that'd make me worry about
> a potential C stack overflow. 

Hm...  On what architectures is there a potential for a C stack
overflow?  I thought that these days, C stack overflows basically didn't
happened, and we only had the max-specpdl-size as a sanity check to
avoid deep lisp backtraces and stuff, but I may well have misunderstood
things.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Noam Postavsky]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Hm...  On what architectures is there a potential for a C stack
> overflow?  I thought that these days, C stack overflows basically didn't
> happened, and we only had the max-specpdl-size as a sanity check to
> avoid deep lisp backtraces and stuff, but I may well have misunderstood
> things.

The default stack size soft ulimit on GNU/Linux seems to be 8MB, Emacs
tries to increase the limit by some fixed amount.  It can be set to
unlimited (I wouldn't expect most users to do that though).  On macOS,
there seems to be a hard limit of 64MB, so setting to unlimited doesn't
work.

Recent changes to Emacs have caused `read' to use more stack space, so
it's definitely possible to hit the limits when reading a deeply nested
structure (see bugs #27571 and #27779).  Not sure how likely it is when
reading deeply nested HTML, but I'm sure if you increase
max-specpdl-size enough with the right HTML page you can manage it :)

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> The default stack size soft ulimit on GNU/Linux seems to be 8MB, Emacs
> tries to increase the limit by some fixed amount.  It can be set to
> unlimited (I wouldn't expect most users to do that though).  On macOS,
> there seems to be a hard limit of 64MB, so setting to unlimited doesn't
> work.

I see.

> Recent changes to Emacs have caused `read' to use more stack space, so
> it's definitely possible to hit the limits when reading a deeply nested
> structure (see bugs #27571 and #27779).  Not sure how likely it is when
> reading deeply nested HTML, but I'm sure if you increase
> max-specpdl-size enough with the right HTML page you can manage it :)

Yeah, and that makes me want to query the user even less.  Emacs: "Do
this thing you have no idea what is?" User: "Yes."  Emacs: *CRASH*.

So perhaps we should just leave things be and not increase the variable
and not prompt the user...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: jidanni@jidanni.org,  30675@debbugs.gnu.org
> Date: Fri, 13 Apr 2018 13:48:22 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'd feel much better with a lower factor, e.g. some value that is just
> > enough to cover this case plus some slack.  Bonus points for providing
> > a defcustom with the factor, so that users could change that.
> 
> The problem is that HTML may nest arbitrarily deep, so there isn't
> really any way to cover this completely.  But the larger the stack size
> is, the lower the possibility for hitting the limit is.
> 
> FWIW, I haven't hit the limit in about a year, so it's rare for people
> to encounter HTML that's that deeply nested.

Which probably means ten-fold increase is too much.  Would increasing
the limit twice fix this problem?  If so, then let's do that in
shr.el.

> why consult the user at all?  :-)

Because there's a possibility of infinite recursion.  So the question
actually boils down to "how much do you trust the code which caused
this?"  And I'm guessing the answer will tend to be NO with each
additional prompt from the same command.

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Which probably means ten-fold increase is too much.  Would increasing
> the limit twice fix this problem?  If so, then let's do that in
> shr.el.

I'm thinking that any increase might be unsafe, and that we should just
leave the variable be.  We don't want to crash an Emacs where the user
has already increased the limit.

> Because there's a possibility of infinite recursion.  So the question
> actually boils down to "how much do you trust the code which caused
> this?"  And I'm guessing the answer will tend to be NO with each
> additional prompt from the same command.

That's true.  But if there's a chance that Emacs may crash by the user
holding down `y' a bit, then that's no fun, either...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> Because there's a possibility of infinite recursion.  So the question
>> actually boils down to "how much do you trust the code which caused
>> this?"  And I'm guessing the answer will tend to be NO with each
>> additional prompt from the same command.
>
> That's true.  But if there's a chance that Emacs may crash by the user
> holding down `y' a bit, then that's no fun, either...

I've now done this on master, so we'll see how this works out in
practice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#30675: Ask the user what to do when shr-make-table: Variable binding depth exceeds max-specpdl-size

[積丹尼 Dan Jacobson]

EZ> some slack.  Bonus points for providing a defcustom with the factor,
EZ> so that users could change that.

We always want to change it just once, to see if that would help reading
the web page we were trying to read.

We don't want to keep it changed, so that random websites go wild
without it asking us.