Bug 211978 – Add a timeout monitor for JSC stress test

• Status: REOPENED
• Component: New Bugs

[Zhifei Fang]

Add a timeout monitor for JSC stress test

[Zhifei Fang]

Created attachment 399544 [details]
Patch

[Keith Miller]

Is this an actual problem in practice? Have we had issues with the JSC internal killer not working?

[Zhifei Fang]

(In reply to Keith Miller from comment #2)
> Is this an actual problem in practice? Have we had issues with the JSC
> internal killer not working?

It looks like yes, see <rdar://problem/61353156>
Though I am not sure why it is not working

[Jonathan Bedard]

Comment on attachment 399544 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399544&action=review

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:259
> +    echo "#{Shellwords.shellescape(@name)} is time out, killing by timeout monitor"

Should be 'has timed out, killing with the timeout monitor'

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:261
> +    sleep 5  # give a grace period

I don't think the comment here is particularly helpful.

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:324
> +            cmd = timeoutMonitorAddon

I feel like we need to replace this with Python in the near future....this feels incredibly hackey.

[Zhifei Fang]

(In reply to Jonathan Bedard from comment #4)
> Comment on attachment 399544 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399544&action=review
> 
> > Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:259
> > +    echo "#{Shellwords.shellescape(@name)} is time out, killing by timeout monitor"
> 
> Should be 'has timed out, killing with the timeout monitor'
> 
> > Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:261
> > +    sleep 5  # give a grace period
> 
> I don't think the comment here is particularly helpful.
> 
> > Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:324
> > +            cmd = timeoutMonitorAddon
> 
> I feel like we need to replace this with Python in the near future....this
> feels incredibly hackey.
agreed, the script writer may need to generate the pure python code, however I am not sure if that can be run on playstation port, maybe firstly we just change the default one

[Zhifei Fang]

I will change this to just kill jsc process instead kill the whole runner script, this should link the reset error handler instead of repeat it in the timeout monitor process

[Zhifei Fang]

Created attachment 401368 [details]
Patch

[Jonathan Bedard]

Comment on attachment 401368 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401368&action=review

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:239
> +        timeout = ENV['JSCTEST_timeout'].to_i + 10 # let jsc timeout handler trigger first

I'm surprised the style-checker is happy with this.

'Seconds' and 'Let jsc timeout...' should have their first leter capitalized

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:259
> +    echo "#{Shellwords.shellescape(@name)} has time out, killing with timeout monitor"

Should be "has timed out"

[Zhifei Fang]

Created attachment 401371 [details]
Patch

[EWS]

Committed r262807: <https://trac.webkit.org/changeset/262807>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401371 [details].

[Radar WebKit Bug Importer]

<rdar://problem/64178437>

[Caio Lima]

This seems to break all JSC queues. I'm explicitly disabling that for Linux, since current timeout monitor script doesn't work there (https://bugs.webkit.org/show_bug.cgi?id=213017).

[WebKit Commit Bot]

Re-opened since this is blocked by bug 213019