Crash on current master branch

Discussions about the development and maturation of the platform code (UXP).
Warning: may contain highly-technical topics.

Moderators: trava90, athenian200

User avatar
__Sandra__
Hobby Astronomer
Hobby Astronomer
Posts: 27
Joined: 2022-05-16, 08:00
Location: Chernihiv, Ukraine

Crash on current master branch

Unread post by __Sandra__ » 2023-04-12, 18:07

I compiled a browser with UXP at "3a6949f398 [network] Improve checks while parsing MIME parameters". It crashes with long work with google maps. When I compiled a browser with UXP at "c95e970df5 Merge pull request 'Update Performance API to User Timing L3'", without increasing kDefaultResourceTimingBufferSize and it crashes just after loading https://maps.google.com/ (wait a full page load). I think size kDefaultResourceTimingBufferSize somehow affects it. But this increase does not solve the problem itself.

Does someone confirm this behavior?

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-12, 18:33

Include a lot more information when you report crashes please. O.S., architecture, crash details at the very least.
If you experience crashes then it's important to include a callstack especially when on a dev branch.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
__Sandra__
Hobby Astronomer
Hobby Astronomer
Posts: 27
Joined: 2022-05-16, 08:00
Location: Chernihiv, Ukraine

Re: Crash on current master branch

Unread post by __Sandra__ » 2023-04-12, 18:48

In most cases, the program simply closes.

System Win 7 x64, PM 32-bit compiled with latest VS 2019

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-12, 18:53

I was able to reproduce it with VS2022 on Win10 64-bit. Note we don't officially support building with VS2019.
It does seem to be related to the performance work so I re-opened Issue #2053 (UXP) with the information I was able to gather.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-12, 19:31

__Sandra__ wrote:
2023-04-12, 18:48
In most cases, the program simply closes.
What you do is you start Visual Studio without source. Then you start the browser you just built but don't do anything yet to crash it. In VS, go to the Debug menu and select "attach to process", select the Pale Moon instance you just started, and then once it's attached, reproduce the crash. The attached debugger will then be able to give you a lot of information about the crash and a full callstack.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
__Sandra__
Hobby Astronomer
Hobby Astronomer
Posts: 27
Joined: 2022-05-16, 08:00
Location: Chernihiv, Ukraine

Re: Crash on current master branch

Unread post by __Sandra__ » 2023-04-12, 19:59

Thanks for the advice! I program microcontrollers and this is a new universe for me :D . I did it too, But this is not very clear to me :D

Code: Select all

>	[Внедренный фрейм] xul.dll!nsWrapperCache::HasWrapperFlag(unsigned int) Строка 301	C++
 	[Внедренный фрейм] xul.dll!nsWrapperCache::IsDOMBinding() Строка 154	C++
 	[Внедренный фрейм] xul.dll!mozilla::dom::CouldBeDOMBinding(nsWrapperCache *) Строка 935	C++
 	[Внедренный фрейм] xul.dll!mozilla::dom::binding_detail::DoGetOrCreateDOMReflector(JSContext * rval, mozilla::dom::PerformanceMark *) Строка 1123	C++
 	[Внедренный фрейм] xul.dll!mozilla::dom::GetOrCreateDOMReflector(JSContext *) Строка 1207	C++
 	[Внедренный фрейм] xul.dll!mozilla::dom::GetOrCreateDOMReflectorHelper<RefPtr<mozilla::dom::PerformanceMark>,1>::GetOrCreate(JSContext *) Строка 1890	C++
 	xul.dll!mozilla::dom::GetOrCreateDOMReflector<RefPtr<mozilla::dom::PerformanceMark>>(JSContext * cx, RefPtr<mozilla::dom::PerformanceMark> & value, JS::MutableHandle<JS::Value> rval, JS::Handle<JSObject *> givenProto) Строка 1912	C++
 	xul.dll!mozilla::dom::PerformanceBinding::mark(JSContext * cx, JS::Handle<JSObject *> obj, mozilla::dom::Performance * self, const JSJitMethodCallArgs & args) Строка 1298	C++
 	xul.dll!mozilla::dom::GenericBindingMethod(JSContext * cx, unsigned int argc, JS::Value * vp) Строка 2930	C++
 	[Внедренный фрейм] mozjs.dll!js::CallJSNative(JSContext *) Строка 238	C++
 	mozjs.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Строка 479	C++
 	mozjs.dll!InternalCall(JSContext * cx, const js::AnyInvokeArgs & args) Строка 524	C++
 	mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Строка 3012	C++
 	mozjs.dll!js::RunScript(JSContext * cx, js::RunState & state) Строка 419	C++
 	mozjs.dll!js::InternalCallOrConstruct(JSContext * cx, const JS::CallArgs & args, js::MaybeConstruct construct) Строка 500	C++
 	mozjs.dll!InternalConstruct(JSContext * cx, const js::AnyConstructArgs & args) Строка 572	C++
 	mozjs.dll!js::ConstructFromStack(JSContext * cx, const JS::CallArgs & args) Строка 608	C++
 	mozjs.dll!js::jit::DoCallFallback(JSContext * cx, js::jit::BaselineFrame * frame, js::jit::ICCall_Fallback * stub_, unsigned int argc, JS::Value * vp, JS::MutableHandle<JS::Value> res) Строка 6034	C++
 	[Внешний код]	
 	[Указанные ниже кадры могут быть неверны или отсутствовать]	Нет данных

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-12, 21:10

Yeah that callstack is the same one I got when I reproduced the problem. It does seem to be performance API related.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

q160765803
Apollo supporter
Apollo supporter
Posts: 32
Joined: 2023-04-13, 07:57

Re: Crash on current master branch

Unread post by q160765803 » 2023-04-13, 08:05

Last edited by q160765803 on 2023-04-13, 09:38, edited 1 time in total.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-13, 08:46

Those don't seem to be directly relevant for this crash... unless you've built with those ported and it solves the issue?
1159003 was actually skipped on purpose because it would likely lead to (severe) memory leaks and potential overreach by allowing profiling of very long sessions.
The other one is just a performance issue we're dodging exactly because we're enforcing an upper bound.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

q160765803
Apollo supporter
Apollo supporter
Posts: 32
Joined: 2023-04-13, 07:57

Re: Crash on current master branch

Unread post by q160765803 » 2023-04-13, 09:52

Moonchild wrote:
2023-04-13, 08:46
Those don't seem to be directly relevant for this crash... unless you've built with those ported and it solves the issue?
1159003 was actually skipped on purpose because it would likely lead to (severe) memory leaks and potential overreach by allowing profiling of very long sessions.
The other one is just a performance issue we're dodging exactly because we're enforcing an upper bound.
Because it returns nullptr without setting error to aRV. Binding code assumes it is safe to use return variable when there's no error.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35473
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Crash on current master branch

Unread post by Moonchild » 2023-04-13, 09:56

Well then the solution would be to set the error, no?
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
__Sandra__
Hobby Astronomer
Hobby Astronomer
Posts: 27
Joined: 2022-05-16, 08:00
Location: Chernihiv, Ukraine

Re: Crash on current master branch

Unread post by __Sandra__ » 2023-04-13, 10:53

I do not understand how the whole system works, but in simple logic, if the crash time is more with a larger buffer, then we put something there but do not remove from there? Is it right at all?

q160765803
Apollo supporter
Apollo supporter
Posts: 32
Joined: 2023-04-13, 07:57

Re: Crash on current master branch

Unread post by q160765803 » 2023-04-13, 10:59

Moonchild wrote:
2023-04-13, 09:56
Well then the solution would be to set the error, no?
yes, this can prevent from crashing, but the site will not work anymore.

Code: Select all

already_AddRefed<PerformanceMark> Performance::Mark(
  JSContext* aCx,
  const nsAString& aName,
  const PerformanceMarkOptions& aMarkOptions,
  ErrorResult& aRv)
{
  // Don't add the entry if the buffer is full. XXX should be removed by bug 1159003.
  if (mUserEntries.Length() >= mResourceTimingBufferSize) {
    aRv.Throw(NS_ERROR_DOM_UT_UNKNOWN_MARK_NAME); // <-- adding this preventing crash
    return nullptr;
  }
I wonder if this array can be replaced with a ring buffer?

EDIT: with somewhat detailed look at Performance.cpp, it seems that deque may be needed instead of simple write index wrapping.

Locked