ChanServ changed the topic of #wayland to: https://wayland.freedesktop.org | Discussion about the Wayland protocol and its implementations, plus libinput | register your nick to speak
rektide has joined #wayland
columbarius has joined #wayland
co1umbarius has quit [Ping timeout: 480 seconds]
Leopold__ has joined #wayland
Leopold_ has quit [Ping timeout: 480 seconds]
Leopold_ has joined #wayland
Leopold__ has quit [Ping timeout: 480 seconds]
adarshgm has joined #wayland
adarshgm has quit [Ping timeout: 480 seconds]
naveenk2 has joined #wayland
adarshgm has joined #wayland
ybogdano has quit [Ping timeout: 480 seconds]
adarshgm has quit [Ping timeout: 480 seconds]
adarshgm has joined #wayland
Company has quit [Quit: Leaving]
jgrulich has joined #wayland
naveenk21 has joined #wayland
naveenk2 has quit [Ping timeout: 480 seconds]
tzimmermann has joined #wayland
Leopold_ has quit [Ping timeout: 480 seconds]
slattann has joined #wayland
dcz_ has joined #wayland
hardening has joined #wayland
luyn has quit [Remote host closed the connection]
luyn has joined #wayland
naveenk21 has left #wayland [#wayland]
mvlad has joined #wayland
GentooPhysicist393542 has quit []
jgrulich has quit [Remote host closed the connection]
<daniels>
DemiMarie: ‘extremely, severely, memory-constrained, to the point where overcommit is disabled, and malloc() may return NULL, and is expected to do do routinely, where you must deal with the allocation failing and not fall over’
<daniels>
(libwayland does not satisfy this constraint)
<pq>
emersion, it tries to not crash at least. But if it needed to not disconnect randomly as well, you'd pretty much need to rewrite it from scratch.
<emersion>
in wlroots, the main goal is to never crash because of OOM
<pq>
There is also the question of which one is better for a use case: limp along with failing random stuff hoping for the best, or crash hard and reboot on signs of unexpected trouble. Of course, what is unexpected rather than expected trouble is a key question.
<pq>
it's good that weston and wlroots have different goals
fahien has joined #wayland
<daniels>
yeah, there’s a huge gulf between ‘has some NULL checks’ and ‘works’
<emersion>
jadahl: i don't get why it would be very annoying to extend?
<emersion>
this is just about the association, so if there is a new way, just define a new creation request?
<jadahl>
i find "create()" then "create2()", then "create3()" with "fixes" to old APIs to be quite terrible
<jadahl>
especially when we can solve it up front
<emersion>
create_with_serial
<emersion>
create_with_other_thing
<jadahl>
right, that "other_thing" might have a serial too
<emersion>
i don't understand
<jadahl>
then the third thing will have three things
<emersion>
if there something replacing the serial, then it won't have a serial in it
<jadahl>
if we have a single request that always creates a finalized object, we have to create a new request, deprecating the old, for every new metadata we want to setup during construcion
<jadahl>
replacing the serial is one thing, adding other useful state is what I'm talking about
<emersion>
we only need the metadata necessary for the association
<jadahl>
that's our current understanding
<jadahl>
maybe that'll change
<emersion>
yeah, and that other state can totally use new requests on the object
<emersion>
without changing the creation request
rasterman has quit [Quit: Gettin' stinky!]
<jadahl>
no we can't, since it has no "I'm ready" request
<jadahl>
how will the compositor know whether to expect set_other_state()?
<emersion>
either with the version, either by tying it to surface commit
<jadahl>
xwl = create_with_serial(wl_surface, serial); wl_surface.commit() works too yes
<jadahl>
that makes it incomptele until the commit
<emersion>
it all depends what that state is
<jadahl>
what I want to avoid is "create_xwayland_surface() { if (version == something) finish() } on_commit() { if (version == something_else() finish() }"
<jadahl>
but instead have bulit into the protocol a "pending" state, that is atomically applied
<emersion>
pending state for something which won't ever change for the lifetime of the object
<emersion>
that's some serious over-engineering here
<jadahl>
I disagree
<jadahl>
it's just sane interface design
<jadahl>
that avoids sprinkeling "if version is this change beavior this way"
<emersion>
i don't expect adding new state which makes a surface "incomplete"
<jadahl>
it might be incomplete in a way that a compositor would make a slightly different decision if it had a complete picture
<jadahl>
it avoids the compositor seeing the "incomplete" association, then after having done a bunch of work with the data it had available, it sees the new "amended state" and it'll have to undo those changes and do the work again
<jadahl>
i don't have explicit ideas for what state is needed, it's just about "interface hygiene" that doesn't lead us into a future corner
fahien has quit [Ping timeout: 480 seconds]
adarshgm1 has joined #wayland
adarshgm has quit [Ping timeout: 480 seconds]
adarshgm1 has quit [Ping timeout: 480 seconds]
adarshgm has joined #wayland
fahien has joined #wayland
<pq>
jadahl, good points. We have many examples of building up state bit by bit, then using it atomically: wl_surface.commit, wl_region, zwp_linux_buffer_params_v1.
Lucretia has quit []
Lucretia has joined #wayland
<jadahl>
yea, it's practically standard practice by now
<zzag>
jadahl: it's awkward that we pass the surface to the factory request, but the serial, which we know won't change, via a separate request; a commit request can be added to make the interface extensible if needed
tlwoerner_ has quit []
tlwoerner has joined #wayland
<zzag>
xwayland_surface.associate is not extensible itself
<zzag>
you cannot add more arguments to it
paulk has joined #wayland
<paulk>
hi folks
<paulk>
just came across an interesting crash in weston
<paulk>
when using the screenshot feature
<paulk>
what happens is that weston crashes when client request a screenshot dies between the shoot request and the frame done notify callback in weston that populates the screenshot data
<paulk>
the reason is that the provided shm buffer is no longer valid, since the program that allocated it was terminated by the kernel (and its ressources with it)
<paulk>
when the client requesting a screenshot dies*
<paulk>
is there a way to know whether such a buffer is still valid before trying to memcpy to it?
<pq>
paulk, how is it possible for the buffer to be deallocated while the compositor has an fd or mmap to it?
<pq>
paulk, the buffer is valid until the compositor closes all fds to it and munmaps all mmaps to it. If it was backed by a file, the file could be truncated, which results in a SIGBUS in the compositor, which Weston has code to handle gracefuly.
<pq>
paulk, is it a SIGBUS or SIGSEGV?
<pq>
or maybe both?
<paulk>
pq: ok I wasn't clear on whether the kernel was taking in account the possibility for another program to have the fd opened / mapped
<emersion>
pq, the wl_buffer resource is no longer around when the client dies
<paulk>
pq: definitely SIGSEGV
<emersion>
the wl_shm_buffer is invalid
<pq>
emersion, oh, this is that issue.
<paulk>
but from time to time I do see the "Buffer address requested when its parent pool has an external reference and a deferred resize pending."
<paulk>
just before weston crashes
<pq>
I didn't think Weston took any external references...
<paulk>
emersion: I tried to add a wl_shm_buffer_get before the memcpy but it looks fine
<pq>
paulk, it could still be freed memory it's looking at
<paulk>
yes I think that's what's happening
<paulk>
is there a way to know that the wl_shm_buffer is no longer valid?
<pq>
libweston/screenshooter.c seems to have a bug: it holds a pointer to struct weston_buffer without listening for its desctruction.
<pq>
That seems to be the answer: listen for the desctruction in addition to holding a pointer.
<paulk>
can that be done in a non-racy way with the frame done listener?
<jadahl>
zzag: its extensible if it's defined to be the "commit". just add more requests before that will later be applied on "associate"
<paulk>
well, non-racy with screenshooter_frame_notify I mean
<jadahl>
zzag: thats how a handful of extensions extend some interface while still making things behave atomically
<zzag>
jadahl: yep, more requests can be added as needed
<zzag>
my only nit is that the surface serial can be passed through get_xwayland_surface, we know that it won't change
fmuellner has quit [Ping timeout: 480 seconds]
<zzag>
you cannot change it similar to wl_surface
<emersion>
… maybe wl_surface shouldn't be an argument and should be a set_surface request ;)
* zzag
regrets mentioning that xD
pw is now known as Guest1644
pw has joined #wayland
fahien has quit [Ping timeout: 480 seconds]
fmuellner has joined #wayland
<daniels>
paulk: you'd need to remove the frame listener if the buffer went away
bluepenquin has joined #wayland
<paulk>
daniels: yeah but I mean, am I guaranteed that I get the destroy notification before the frame listener?
<paulk>
what if the client dies when I'm already in the frame listener
<daniels>
paulk: that's fine if the client dies whilst you're in the frame listener - the wl_shm_buffer and its mapping remain valid right up until destroy_listener fires