[Webkit-unassigned] [Bug 184039] webkitpy: Implement coredumpctl support on linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=184039

--- Comment #5 from Thibault Saunier <tsaunier at gnome.org> ---
(In reply to Daniel Bates from comment #3)
> Comment on attachment 336581 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=336581&action=review
> 
> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:32
> > +import datetime
>
> Not in sorted order. Please sort all the  import lines suchthat they are in
> sorted order according to the UNIX sort tool.

Done.

> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:60
> > +        for ntry in range(5):
> 
> This is a suboptimal way to loop as we malloc a list. What does ntry stand
> for? Did you mean to write retries?

s/ntry/try_number/ (0 is not a retry). 

> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:62
> > +                # Loopping, it means we conceder the logs might not be ready
> 
> Please remove this comment as it is redundant given the comment on line 59.
> Also, I am surprised we don’t have a convenience function in webktpy to do
> waiting for us.

Done - how more convenient than that could it be? (I have not found anything special, but might be wrong).

> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:64
> > +                time.sleep(1)
> 
> Is there not a more direct way to know if systemd generating the backtrack
> so we can avoid busy waiting?

Well, systemd can't know if it is going to generate a trace it might not now about yet (apart from doing what we do "wait and see") - I checked the CLI but couldn't see anything better.

> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:67
> > +                info = subprocess.check_output(['coredumpctl', 'info',
> 
> Can we write this logic using the Executive object? Can this class take an
> Executive object in its constructor?

Done. And moved all the Popen calls to using it.

> > Tools/Scripts/webkitpy/port/linux_get_crash_log.py:71
> > +                # The trace might not be ready yet
> 
> :( Can we find a more deterministic and direct way to test this. Writing
> code that sleeps and hopes the data becomes available when we wake
> artificially slows down testing.

Yeah it is not optimal but I don't think we can do anything better. Ideally that code paths should never be reached (generating backtraces for crashes)... and in practice it is not going to happen often (and in my limited tested, and on my quite powerful machine, retrying won't ever happen more than once, so 1sec lost in the worst case of the worst case).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180329/d25abe58/attachment.html>