On Thu, Sep 17, 2015 at 4:54 PM, Marc-André Lureau <mlureau@xxxxxxxxxx> wrote:
----- Original Message -----
> > > Sorry I didn't mean what the thread was doing, but rather why we need a
> > > thread for this task.
> >
> > Well I could use a g_task or g_idle, or something like that, but g_idle
> > isn't
> > cancellable, and g_task
>
> g_idle is cancellable.
>
> Hmm, I couldn't find in the documentation. Usually the functions have some
> kind of GCancellable parameter.
You can do g_source_remove(), no need for cancellable
Ah, ok.
> It's hardly convincing me, threads are more racy by nature.
>
> That's true, but since I just need a few lines of code in the thread, and
> it's not
> accessing anything externally (other then the cancel), the thread API is
> simpler for what I need.
>
> With glib functions (g_task, g_idle, GSimplaAsyncResult), I need to supply
> stuff
> I either don't need, or I don't know what they're for. I just need something,
> that will
> call a function after 5 seconds, and can be cancelled.
> That's 2-3 parameters (callback, cancel, maybe time).
>
> Since I don't have that much experience with GLibs Async functions, I chose a
> thread.
> For me it's simpler to implement and to test, it's fewer lines of code, and
> since the function
> it runs is simple, I don't think that there is any advantage in using
> something else.
> More so, if the other functions spawn a thread internally anyway.
>
> I tried using a timer, and removing it before it fired, but sometimes it
> fired, even when it wasn't supposed to.
> I could pass a GCancellable to the timeout instead, and let it fire, but then
> I would have multiple timeouts
> firing at the same time, for no reason (because I would cancel the
> cancellable).
>
not clear to me, but ok
>
>
> Trying to read from the driver when the started is wrong, at least it should
> be after the virtio and the listening socket have been successfully opened.
>
> Well I'm not actually reading anything. And I think that read_thread starts
> only after
> everything is prepared, because If I start the service with sharing enabled,
> everything works fine.
> If the user is not connected/sharing is not enabled, the only thing my
> patches do, is that the new thread
> waits for a few seconds, and gets cancelled, when the main loop ends (which
> will happen almost immediately).
Yes, so you could start it after the service is actually started and the virtio port is opened, no? (repeating myself here)
Ah you mean socket_service? I thought you meant the windos service.
Yeah I can move it just above start_mux_read (mux_istream);
Yeah I can move it just above start_mux_read (mux_istream);
>
> I can imagine it may block because WNetAddConnection2 function call is sync
> and will probe the service. But if it's blocking then it will not be able to
> handle its communication work.
>
> Why not wrap WNetAddConnection2 in a GSimplaAsyncThread and run it once the
> client connection is established?
>
> Well that's the problem. I don't know when the client is connected, because
> there is no signal/callback,
> anything that would notify me, that the client connected. The only way I
> know, that the client is connected,
> is that the service stops looping, because it blocked in read_thread (this
> was happening before my patches also).
>
> When it blocks, I have no way to call anything, that would map the drive, or
> notify something to map the drive.
read_async is.. async! the thread is a helper, the mainloop is free for other events/task.
But read_async is not the issue, read_thread is.
g_input_stream_read_all() is not async, so it blocks when there are no data. It only receives data, after:
g_input_stream_read_all() is not async, so it blocks when there are no data. It only receives data, after:
1. client connects
2. enables sharing
3. maps the drive
> That's why the only way was to start something before the read blocked, wait
> for a few seconds, and if nothing
> told it otherwise, it would map the drive. If the read didn't block, it would
> somehow notify this something,
> that the client is not actually connected, and not to map the drive.
>
> I spent a few weeks trying to implement something using already defined
> signals, g_poll, even windows specific
> polling/async reading, but nothing worked.
Check if the mainloop is working properly using a timer, it should be fine. Then we can realize it's blocking when you actuall call WNetAddConnection2, and then you realize it's the reason you need a thread.
The mainloop should work fine, the only think that's blocked is the read_thread
(talking about a function called read_thread here)
(talking about a function called read_thread here)
I can try something else using mainloop, but just so we understand each other,
I still need another thread. It doesn't matter, if it's mainloop, or mine thread,
but it HAS to be a different thread then the one where read_thread() is working.
We agree on that right?
I still need another thread. It doesn't matter, if it's mainloop, or mine thread,
but it HAS to be a different thread then the one where read_thread() is working.
We agree on that right?
>
> The bottom line is that even when it is usually better to use something more
> high level then a thread,
> I think it's safe to use a thread here, and using more high level stuff
> wouldn't bring any advantage.
My bottom line is: don't use thread if you don't need them
> When I tried implementing something other then a thread, the only thing that
> brought was a headache,
> because they refused to work at all.
That's a good sign that there is something you didn't solve properly.
Yes, I'm aware of that. But since thread worked properly, I had no issue with using it.
>
>
>
> > As for why this has to be in a different thread, then read_thread, is
> > because
> > if I call map_drive before
> > read actually blocks, it returns error, as if it's not connected. I think
> > this is because when we try to map
> > the drive, client sends a message to the guest, and the read has to be
> > ready
> > to receive it, which in turn
> > lets the map_drive to actually map the drive.
> >
> > I tryed using every possible way I could think of, even windows specific
> > polling, but I couldn't make
> > anything work, except just plain spawning another thread.
> >
> > Lukas Venhoda
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
>
> Lukas Venhoda
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
Lukas
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel