GNOME Bugzilla – Bug 671139
need (transfer async) for io stream buffers
Last modified: 2015-02-07 16:48:59 UTC
bug 669306 shows g_output_stream_write_async() not working from python, because the buffer gets garbage collected after calling g_output_stream_write_async(), but before the stream actually gets a chance to write from it. There's no way to deal with this with the current set of transfer annotations; (transfer none) is what it is now, and (transfer full) would cause it to be leaked. We need something like "(transfer async)", to say that the data must remain valid until the callback is called back. Or maybe something else? Actually, g_input_stream_read_async() is probably going to be problematic for lots of bindings even with this...
oops, wrong component
i've always hated this API for exactly this reason i'd almost consider deprecating it or skipping it as an alternative...
well... you want there to be some way to do async I/O... but we could add new methods and Rename: them over the old ones. For write, we could just make the buffer (transfer full), but there's no correspondingly simple fix for read.
Shouldn't we just (skip) G*Stream API and recommend users to use GDataInputStream which is more binding friendly?
GDataInputStream currently only has "read up until such and such a character" APIs; we'd need to add a "read up to this many bytes" API. But that doesn't help with the output side though. I guess we could add a (transfer full) "g_output_stream_take_and_write()", and Rename: it over g_output_stream_write()... Hm... or maybe: void g_input_stream_read_bytes_async (GInputStream *stream, gsize count, gint io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); GBytes *g_input_stream_read_bytes_finish (GInputStream *stream, GAsyncResult *result, GError **error); void g_output_stream_write_bytes_async (GOutputStream *stream, GBytes *data, gint io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); gssize g_input_stream_write_bytes_finish (GOutputStream *stream, GAsyncResult *result, GError **error);
Created attachment 212624 [details] [review] gio: add GBytes-based input/output stream methods Using a caller-supplied buffer for g_input_stream_read() doesn't translate well to the semantics of many other languages, and using a non-refcounted buffer for read_async() and write_async() makes it impossible to manage the memory correctly currently in garbage-collected languages. Fix both of these issues by adding a new set of methods that work with GBytes objects rather than plain buffers. ==== needs tests, but what do people think? In particular, would this actually be more usable for bindings (once bindings learn about GBytes)?
*** Bug 674901 has been marked as a duplicate of this bug. ***
Yeah, this is an embarassing gap in our stack. It might be possible to try to annotate this somehow (out caller-allocates)? And teach bindings how to handle this for arrays. Note for some bindings, it may be possible to just hand down a pointer to their underlying array storage, but for others they may need to allocate a temporary, then copy.
*** Bug 676743 has been marked as a duplicate of this bug. ***
Review of attachment 212624 [details] [review]: This doesn't build. Attaching a patch that does. ::: gio/goutputstream.c @@ +330,3 @@ +{ + return g_output_stream_write (stream, + g_bytes_get_data (bytes), get_data takes an out pointer to a size. @@ +924,3 @@ + (GDestroyNotify) g_bytes_unref); + + g_input_stream_write_async (stream, input_stream_write_async? @@ +925,3 @@ + + g_input_stream_write_async (stream, + g_bytes_get_data (bytes), get_data takes an out pointer to a size. @@ +952,3 @@ + GSimpleAsyncResult *simple; + + g_return_val_if_fail (G_IS_OUTPUT_STREAM (stream), NULL); NULL is not a gssize. @@ +953,3 @@ + + g_return_val_if_fail (G_IS_OUTPUT_STREAM (stream), NULL); + g_return_val_if_fail (g_simple_async_result_is_valid (result, G_OBJECT (stream), g_output_stream_write_bytes_async), NULL); NULL is not a gssize.
Created attachment 214873 [details] [review] gio: add GBytes-based input/output stream methods Using a caller-supplied buffer for g_input_stream_read() doesn't translate well to the semantics of many other languages, and using a non-refcounted buffer for read_async() and write_async() makes it impossible to manage the memory correctly currently in garbage-collected languages. Fix both of these issues by adding a new set of methods that work with GBytes objects rather than plain buffers.
Review of attachment 214873 [details] [review]: Did you actually test this one? =) If we're aiming for introspection usability, we need to make GBytes work in gjs first, so we know this API is sane. Also this does need: 1) Tests in GLib too; there's gio/tests/memory-input-stream.c where I just added some other GBytes-based stuff. 2) New symbols added to gio/gio.symbols 3) New symbols added to docs/reference/gio/gio-sections.txt ::: gio/ginputstream.c @@ +277,3 @@ + * g_input_stream_read_bytes: + * @stream: a #GInputStream. + * @count: the number of bytes that will be read from the stream I suppose this bit just got copied from g_input_stream_read(), but we should probably mention something about the recommended size. The classic ones are 4096 and 8192. I'd phrase this as: @count: maximum number of bytes to be read from the stream @@ +305,3 @@ + * On error %NULL is returned and @error is set accordingly. + * + * Return value: a new #GBytes, or %NULL or %NULL on error ::: gio/ginputstream.h @@ +124,3 @@ gsize *bytes_read, GCancellable *cancellable, GError **error); GLIB_AVAILABLE_IN_2_34
Created attachment 214896 [details] [review] gio: add GBytes-based input/output stream methods Using a caller-supplied buffer for g_input_stream_read() doesn't translate well to the semantics of many other languages, and using a non-refcounted buffer for read_async() and write_async() makes it impossible to manage the memory correctly currently in garbage-collected languages. Fix both of these issues by adding a new set of methods that work with GBytes objects rather than plain buffers. This should fix most issues.
Review of attachment 214896 [details] [review]: This looks good, but before you commit can you link to the gjs patch?
Review of attachment 214896 [details] [review]: What gjs patch?
(In reply to comment #15) > Review of attachment 214896 [details] [review]: > > What gjs patch? Exactly =) I'm worried about landing new GIO with the express purpose of making it work in language bindings, but then finding out it doesn't actually work... I can do the GBytes<->bytearray patch for gjs if you want.
Review of attachment 214896 [details] [review]: Turns out we don't need a gjs patch.
Attachment 214896 [details] pushed as 800d6ff - gio: add GBytes-based input/output stream methods
*** Bug 621915 has been marked as a duplicate of this bug. ***
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]