> > On Thu, Aug 31, 2017 at 02:07:14PM +0100, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > .gitignore | 1 + > > docs/Makefile.am | 6 ++-- > > docs/spice_threading_model.txt | 73 > > ++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 78 insertions(+), 2 deletions(-) > > create mode 100644 docs/spice_threading_model.txt > > > > diff --git a/.gitignore b/.gitignore > > index 25db761a..bf618932 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -35,6 +35,7 @@ INSTALL > > docs/manual/manual.chunked/ > > docs/manual/manual.html > > docs/spice_style.html > > +docs/spice_threading_model.html > > .dirstamp > > .deps > > .libs > > diff --git a/docs/Makefile.am b/docs/Makefile.am > > index 3dba2c3a..45667a60 100644 > > --- a/docs/Makefile.am > > +++ b/docs/Makefile.am > > @@ -4,14 +4,16 @@ ASCIIDOC_FLAGS = -a icons -a toc > > EXTRA_DIST = \ > > spice_style.html \ > > spice_style.txt \ > > + spice_threading_model.html \ > > + spice_threading_model.txt \ > > $(NULL) > > > > if BUILD_MANUAL > > SUBDIRS = manual > > > > -all-local: spice_style.html > > +all-local: spice_style.html spice_threading_model.html > > > > -spice_style.html: spice_style.txt > > +%.html: %.txt > > $(AM_V_GEN) $(ASCIIDOC) -n $(ASCIIDOC_FLAGS) -o $@ $< > > endif > > > > diff --git a/docs/spice_threading_model.txt > > b/docs/spice_threading_model.txt > > new file mode 100644 > > index 00000000..f88ead6d > > --- /dev/null > > +++ b/docs/spice_threading_model.txt > > @@ -0,0 +1,73 @@ > > +How does SPICE handle threading > > +------------------------------- > > + > > +Lot of code run in a single thread. > > "Lots of code" > > > + > > +Qemu usually calls SPICE from its main thread except on call backs to code > > +already running in different threads. > > I think it's "callbacks", but not sure. > > > + > > +Channels run mostly on a single thread except on creation and destruction > > which > > +MUST be done in the main thread. > > I'd be a bit more explicit about "Channels" at least here: > "Channels (RedChannel/RedChannelClient code) run mostly in a single > thread..." (ok, you detail this a bit down the text) > > > +Lots of channels run in the main thread but currently CursorChannel and > > +DisplayChannel can be run from within a thread created by RedWorker. > > "can be run" -> "are run"? > > > +Note that different CursorChannel/DisplayChannel (they differ by id) run > > in > > +separate RedWorker threads (currently a single RedWorker thread run a pair > > "runs" > > > +of CursorChannel and DisplayChannel). > > + > > +RedChannelClient runs in the RedChannel thread. Creation is done in the > > same > > +thread while destruction can happen in different threads. > > + > > +A RedClient instance groups the various RedChannelClient connected to > > +the same remote SPICE client. > > +These RedChannelClient instances can run in separate threads: > > +MainChannelClient and most of the other RedChannelClient will be > > +running in the main thread, while the CursorChannelClient and the > > +DisplayChannelClient usually will be running from a different thread. > > + > > +Another important aspect of dealing with multiple threads are the > > dispatchers. > > +Dispatchers are used to send messages/request from one thread to another. > > +The main dispatcher is used to send requests to the main thread. > > +The Qxl uses a dispatcher to send requests to the RedWorker which will > > forward > > +to DisplayChannel/CursorChannel. > > For what it's worth, I would not call this an "important aspect", more > of an uninteresting implementation detail :) > I think I'd describe this as a way of ensuring a method is called in a > different thread, either synchronously or asynchronously. It's more > explicit than "sending requests" > I think is pretty important. Lots of implementations uses mostly mutexes while in SPICE code there's a lot of requests between threads. > > + > > +RedClient may call some RedChannelClient functions using some callbacks > > +registered inside ClientCbs. Usually these callbacks are functions that do > > the > > +job directly if the RedChannel is running in the main thread or they use a > > +dispatcher to do the job in the right thread. Currently there are 3 > > callbacks: > > +connect, disconnect and migrate. Connect and migrate are asynchronous (the > > job > > +is done while the current thread is doing something else) while disconnect > > is > > +synchronous (the main thread will wait for termination). > > + > > +Reference counting and ownership > > +-------------------------------- > > +-> pointer > > + > > +=> pointer with owning (mostly GObject ref counting) > > + > > +RedChannelClient::client -> RedClient > > + > > +RedClient::channels => RedChannelClient > > + > > +RedClient::mcc -> MainChannelClient > > + > > +RedChannelClient::channel => RedChannel > > + > > +RedChannel::clients -> RedChannelClient > > + > > +The RedClient represents a connection from a remote SPICE client, > > +RedClient::channels corresponding to the connections for the individual > > +channels. Their lifetime is tied to the TCP connection to this remote > > client. > > +when a disconnection is detected, the corresponding RedChannelClient are > > "When" > > > +removed from RedClient::channels and Channel::clients (the main owner of > > I guess should be "RedChannel", but fairly obvious given all the > surrounding text. > > > +RedChannelClient is RedClient). > > +When MainChannelClient is disconnected all RedChannelClients connected to > > the > > +same RedClient are automatically disconnected. > > "is disconnected, all RedChannelClients" > > > + > > +Who owns RedClient? RedClient is released when the MainChannelClient > > attached > > +to it is disconnected. In this case a request is scheduled in the main > > thread > > +(even if MainChannelClient runs on the main thread too) and > > +red_client_disconnect is called which calls red_client_destroy which uses > > +g_object_unref to free the object. > > + > > +Where is freed the MainChannelClient? On disconnection like other channel > > I believe "Where is the MainChannelClient freed?" sounds better/is more > correct > > Overall looks good! > > > +clients, currently before RedClient which will have a dangling pointer. Hope I got all the other comments right. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel