Re: MPS: Win64 testers?

[Eli Zaretskii <eliz@gnu.org>]

> Date: Fri, 23 Aug 2024 18:57:12 +0000
> From: Pip Cet <pipcet@protonmail.com>
> Cc: sebastian@sebasmonia.com, kien.n.quang@gmail.com, emacs-devel@gnu.org, yantar92@posteo.net
> 
> "Eli Zaretskii" <eliz@gnu.org> writes:
> 
> >> If your point is that it might be a long time until MPS is stable enough
> >> for very unusual configurations, that is certainly true, and affects
> >> when it's a good idea to enable it for general use.
> >
> > Not "for very unusual configurations", for all configurations,
> > including the most popular ones.
> 
> Which ones do you mean?

All of them.  I'm quite sure we have a lot of corners the few people
who are using the branch have not entered yet, especially as some bugs
don't happen until GC hits at just the right (or wrong) moment.  Like
this GnuTLS crash, for example.

> > We are touching the most delicate
> > parts of Emacs, and will probably touch a lot of them (unless we find
> > some simpler, more reliable way of preventing these problems).
> 
> Simpler, maybe, but reliability, I will keep insisting, improves if we
> think about why it's safe to hand off a pointer to GCable memory to an
> externable library.

You can insist all you want, but you will never convince me.  I have
too much gray hair from opinions based on the assumption that we
completely understand what we are doing by such changes and what they
cause in Emacs.  Just stick long enough here and you will see another
example of this fallacy almost every day.

So no, IME reliability definitely does NOT improve by futzing with
this kind of stuff.  Which means we should not do it unless we must.

> 
> >> >> This particular call is safe.  'match_data' lives on the stack, MPS
> >> >> treats the stack conservatively as pinning every object to which it
> >> >> might possibly refer, thus 'match_data->pattern' cannot be moved.
> >> >
> >> > IOW, we are lucky.  Because match_data could well be a global variable
> >> > allocated off the heap, or something else.
> >>
> >> Which would be a bug both with and without MPS, as nothing protects
> >> match_data.list from GC if it doesn't live on the stack.
> >
> > ??? We have gobs of those, and know how to protect them.  The easiest
> > way is staticpro, and there are others.
> 
> My point is you'd have to protect it, albeit slightly differently, with
> or without MPS.  Not that much changes with MPS, and to say it works
> because "we are lucky" is inaccurate.

"Not that much changes" is IMO an understatement of the decade.
Compare the simplicity of staticpro with the complicated dance you
suggested for this GnuTLS issue (which is not different from many
similar changes installed on the branch each time we detect some
similarly "unsafe" use of data structures).  One can use staticpro
without even fully understanding how it works and why.  We have
nothing comparable on the MPS branch, and thus no established
procedure for solving such problems or rules for auditing code for
similar issues.

> >> Better to solve a problem twice than not to solve it at all?
> >
> > Not in my book, no.  Since Emacs is stable, we should not add any code
> > that is not strictly necessary, otherwise we risk unnecessary
> > problems.
> 
> The master branch may be, but there's nothing wrong with keeping our
> options open on scratch/igc rather than leaving in a few "redundant"
> safety checks.

If we want this branch to ever land on master, we had better handled
it with the same care as we do on master.  Because when we decide to
land it, no one will have time or motivation to go through all this
stuff and "fix" it.  We want Emacs to be as stable the day after this
lands as the day it was before that.  That's how it was for all the
other major features that got merged from feature branches.  Any
significant drop in stability as result of the merge will be a net
loss.