Bug 62372 - [PATCH] When sleeping in the example provider, deal with immediate cancellation.
Summary: [PATCH] When sleeping in the example provider, deal with immediate cancellation.
Status: RESOLVED FIXED
Alias: None
Product: realmd
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Stef Walter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 62214
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-15 15:10 UTC by Marius Vollmer
Modified: 2013-04-16 21:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
When sleeping in the example provider, deal with immediate cancellation. (5.42 KB, patch)
2013-03-15 15:10 UTC, Marius Vollmer
Details | Splinter Review
When sleeping in the example provider, deal with immediate cancellation. (4.91 KB, patch)
2013-04-15 12:35 UTC, Marius Vollmer
Details | Splinter Review

Description Marius Vollmer 2013-03-15 15:10:52 UTC
Created attachment 76562 [details] [review]
When sleeping in the example provider, deal with immediate cancellation.

When sleeping in the example provider, deal with immediate
cancellation.
    
    * service/realm-example-provider.c (on_discover_sleep_done): Always
      complete in idle, as expected.
    
    * service/realm-usleep-async.c (realm_usleep_async): Check for
      immediate cancellation, to avoid calling on_sleep_async_cancelled
      before we are done initializing.  Also, change ownership handling to
      be more straightforward and to allow both timeout and cancelling to
      occur at the same time.
Comment 1 Stef Walter 2013-03-22 13:46:14 UTC
Comment on attachment 76562 [details] [review]
When sleeping in the example provider, deal with immediate cancellation.

Review of attachment 76562 [details] [review]:
-----------------------------------------------------------------

::: service/realm-example-provider.c
@@ +99,4 @@
>  	if (!realm_usleep_finish (res, &error))
>  		g_simple_async_result_take_error (async, error);
>  
> +	g_simple_async_result_complete_in_idle (async);

Is this because realm_usleep_async() sometimes calls its GAsyncResultReady callback immediately? If so, then its breaking the gio async style guarantees (a GAsyncReadyCallback should only ever be called after returning to the main loop) and the bug should be fixed in realm_usleep_async() and not worked around here.
Comment 2 Marius Vollmer 2013-04-12 10:02:09 UTC
(In reply to comment #1)
> > +	g_simple_async_result_complete_in_idle (async);
> 
> Is this because realm_usleep_async() sometimes calls its GAsyncResultReady
> callback immediately?

Yes.

> If so, then its breaking the gio async style
> guarantees (a GAsyncReadyCallback should only ever be called after returning
> to the main loop) and the bug should be fixed in realm_usleep_async() and
> not worked around here.

Aha!  I didn't know about this guarantee.

Would you say that realm_usleep_async now does the right thing, with this patch?
Comment 3 Stef Walter 2013-04-12 12:04:05 UTC
This doesn't apply to git master, realm-usleep-async.c has lots of conflicts. Does it depend on another patch?
Comment 4 Marius Vollmer 2013-04-15 12:35:24 UTC
(In reply to comment #3)
> This doesn't apply to git master, realm-usleep-async.c has lots of
> conflicts. Does it depend on another patch?

No, I think it was just some minor formatting differences.
Comment 5 Marius Vollmer 2013-04-15 12:35:56 UTC
Created attachment 77992 [details] [review]
When sleeping in the example provider, deal with immediate cancellation.
Comment 6 Stef Walter 2013-04-16 21:14:20 UTC
Attachment 77992 [details] pushed as 36b1f25 - When sleeping in the example provider, deal with immediate cancellation.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.