Ticket #130 (closed enhancement: fixed)

Opened 7 months ago

Last modified 5 months ago

patch for libsoup 2.4 support

Reported by: danw Assigned to: bellmich
Priority: highest Component: Transports
Version: 0.4.6 Severity: normal
Keywords: Cc: danw, bellmich, felixmoeller

Description

GNOME 2.22 will have a new API-incompatible version of libsoup. The first beta tarball release of libsoup 2.4 is now available on ftp.gnome.org (http://ftp.gnome.org/pub/GNOME/sources/libsoup/2.3/libsoup-2.3.0.1.tar.bz2).

I'm attaching a patch that makes libsyncml support either libsoup 2.2 or libsoup 2.4 (preferring 2.4 if both are available). It compiles but is completely untested, because I don't have anything to test it with...

Some notes on the patch:

  1. I've never used CMake before, I probably did the tests in an icky way
  2. smlTransportHttpClientFinalize was doing really wacky stuff. I changed it to just call soup_session_abort() (which means that if there are any pending requests, they will immediately finish and invoke _msgReceived with a status of SOUP_STATUS_CANCELLED), and updated _msgReceived accordingly. _msgReceived is probably leaking memory actually (env->data), but that was true before as well.
  3. SoupServer? has changed enough that I ended up having completely separate _server_callback() methods for 2.2 and 2.4. You might decide to try to clean that up a little...
  4. I added a FIXME to the 2.4 _server_callback(), which also applies to the 2.2 one: it causes a leak on both the client and server side. Someone needs to figure out what's actually going on there.
  5. AFAICT, smlTransportHttpServerFinalize was leaking the server. I uncommented the unref.
  6. I added a FIXME to smlTransportHttpServerSend() suggesting that setting the Accept header is probably a bug. Remove the comment or the code accordingly.

Attachments

libsyncml-libsoup24.diff (17.4 kB) - added by danw on 01/29/08 00:35:49.
libsoup 2.4 patch

Change History

01/29/08 00:35:49 changed by danw

  • attachment libsyncml-libsoup24.diff added.

libsoup 2.4 patch

02/11/08 23:20:57 changed by dgollub

  • cc changed from danw to danw, bellmich.
  • priority changed from normal to highest.
  • status changed from new to assigned.

Patch looks great!

I guess some further adjustment are required to make libsyncml compatible with libsoup 2.2 and 2.4 ...

Michael, what do you think about all the mutex changes?

02/12/08 16:07:03 changed by dgollub 

(process:26493): libsoup-CRITICAL **: soup_message_set_status: assertion `SOUP_IS_MESSAGE (msg)' failed

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb7088b90 (LWP 26497)]
0xb7c6cb91 in soup_message_headers_append (hdrs=0xaaaaaaaa, name=0xb7f35e8d "Accept", 
    value=0xb7f35b75 "application/vnd.syncml+wbxml") at soup-message-headers.c:119
119             g_array_append_val (hdrs->array, header);
(gdb) bt
#0  0xb7c6cb91 in soup_message_headers_append (hdrs=0xaaaaaaaa, name=0xb7f35e8d "Accept", 
    value=0xb7f35b75 "application/vnd.syncml+wbxml") at soup-message-headers.c:119
#1  0xb7f26ada in smlTransportHttpServerSend (userdata=0x8bfe0d8, linkdata=0x8c091d0, data=0x8c0ab48, error=0x0)
    at /home/dani/projects/libsyncml/trunk/libsyncml/transports/http_server.c:375
#2  0xb7f0cb48 in smlTransportWorkerHandler (message=0x8c10800, userdata=0x8bfa898)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_transport.c:99
#3  0xb7f0684e in smlQueueDispatch (queue=0x8bfa8e0) at /home/dani/projects/libsyncml/trunk/libsyncml/sml_queue.c:452
#4  0xb7f05587 in _queue_dispatch (source=0x8c02ee8, callback=0, user_data=0x8bfa8e0)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_queue.c:66
#5  0xb7e564b8 in IA__g_main_context_dispatch (context=0x8bfb6f8) at gmain.c:2064
#6  0xb7e59b73 in g_main_context_iterate (context=0x8bfb6f8, block=1, dispatch=1, self=0x8bfb810) at gmain.c:2697
#7  0xb7e5a092 in IA__g_main_loop_run (loop=0x8bfb7f8) at gmain.c:2905
#8  0xb7f0c37a in smlThreadStartCallback (data=0x8bfb780)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_support.c:391
#9  0xb7e80a9f in g_thread_create_proxy (data=0x8bfb810) at gthread.c:635
#10 0xb7a35155 in start_thread () from /lib/libpthread.so.0
#11 0xb7b3d6ce in clone () from /lib/libc.so.6

Looks like the SoupMessage? runs out of references...

All threads: 

(gdb) thread apply all bt

Thread 4 (Thread 0xb6887b90 (LWP 26498)):
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7a38bf5 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
#2  0xb7b49b4d in pthread_cond_wait () from /lib/libc.so.6
#3  0xb7e57a8f in IA__g_main_context_wait (context=0x8bfb568, cond=0x8bfeed8, mutex=0x8bfb56c) at gmain.c:2261
#4  0xb7e59fa2 in IA__g_main_loop_run (loop=0x8bfee00) at gmain.c:2876
#5  0xb7f0c37a in smlThreadStartCallback (data=0x8bfed88)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_support.c:391
#6  0xb7e80a9f in g_thread_create_proxy (data=0x8bfee18) at gthread.c:635
#7  0xb7a35155 in start_thread () from /lib/libpthread.so.0
#8  0xb7b3d6ce in clone () from /lib/libc.so.6

Thread 3 (Thread 0xb7088b90 (LWP 26497)):
#0  0xb7c6cb91 in soup_message_headers_append (hdrs=0xaaaaaaaa, name=0xb7f35e8d "Accept", 
    value=0xb7f35b75 "application/vnd.syncml+wbxml") at soup-message-headers.c:119
#1  0xb7f26ada in smlTransportHttpServerSend (userdata=0x8bfe0d8, linkdata=0x8c091d0, data=0x8c0ab48, error=0x0)
    at /home/dani/projects/libsyncml/trunk/libsyncml/transports/http_server.c:375
#2  0xb7f0cb48 in smlTransportWorkerHandler (message=0x8c10800, userdata=0x8bfa898)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_transport.c:99
#3  0xb7f0684e in smlQueueDispatch (queue=0x8bfa8e0) at /home/dani/projects/libsyncml/trunk/libsyncml/sml_queue.c:452
#4  0xb7f05587 in _queue_dispatch (source=0x8c02ee8, callback=0, user_data=0x8bfa8e0)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_queue.c:66
#5  0xb7e564b8 in IA__g_main_context_dispatch (context=0x8bfb6f8) at gmain.c:2064
#6  0xb7e59b73 in g_main_context_iterate (context=0x8bfb6f8, block=1, dispatch=1, self=0x8bfb810) at gmain.c:2697
#7  0xb7e5a092 in IA__g_main_loop_run (loop=0x8bfb7f8) at gmain.c:2905
#8  0xb7f0c37a in smlThreadStartCallback (data=0x8bfb780)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_support.c:391
#9  0xb7e80a9f in g_thread_create_proxy (data=0x8bfb810) at gthread.c:635
#10 0xb7a35155 in start_thread () from /lib/libpthread.so.0
#11 0xb7b3d6ce in clone () from /lib/libc.so.6

Thread 2 (Thread 0xb7889b90 (LWP 26496)):
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7b33ae7 in poll () from /lib/libc.so.6
#2  0xb7e59a02 in g_main_context_iterate (context=0x8bfb568, block=1, dispatch=1, self=0x8bfb5f0) at gmain.c:3006
#3  0xb7e5a092 in IA__g_main_loop_run (loop=0x8bf9f40) at gmain.c:2905
#4  0xb7f0c37a in smlThreadStartCallback (data=0x8bfb528)
    at /home/dani/projects/libsyncml/trunk/libsyncml/sml_support.c:391
#5  0xb7e80a9f in g_thread_create_proxy (data=0x8bfb5f0) at gthread.c:635
#6  0xb7a35155 in start_thread () from /lib/libpthread.so.0
#7  0xb7b3d6ce in clone () from /lib/libc.so.6

Thread 1 (Thread 0xb788bb80 (LWP 26493)):
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7a3b88c in __lll_unlock_wake () from /lib/libpthread.so.0
#2  0xb7a37feb in _L_unlock_90 () from /lib/libpthread.so.0
#3  0xb7a37c2c in __pthread_mutex_unlock_usercnt () from /lib/libpthread.so.0
#4  0xb7b49d56 in pthread_mutex_unlock () from /lib/libc.so.6
#5  0xb7e586ad in IA__g_source_attach (source=0x8c04d10, context=0x8bfb56c) at gmain.c:934
#6  0xb7f0c63c in smlThreadStop (thread=0x8bfed88) at /home/dani/projects/libsyncml/trunk/libsyncml/sml_support.c:430
#7  0xb7f0d078 in smlTransportStop (tsp=0x8bfb4c0) at /home/dani/projects/libsyncml/trunk/libsyncml/sml_transport.c:217
#8  0x0804a826 in http_send (_i=0) at /home/dani/projects/libsyncml/trunk/tests/check_http.c:351
#9  0xb7bb75d9 in srunner_run_all () from /usr/lib/libcheck.so.0
#10 0x0804e3c5 in main () at /home/dani/projects/libsyncml/trunk/tests/check_http.c:983
0xb7c6cb91      119             g_array_append_val (hdrs->array, header);

This happens with the http_send testcase from the http unittest. Danw, can you reproduce this issue on your side?

02/29/08 15:22:53 changed by felixmoeller

  • cc changed from danw, bellmich to danw, bellmich, felixmoeller.

03/11/08 21:05:01 changed by bellmich

  • owner changed from dgollub to bellmich.
  • status changed from assigned to new.

The patch was modified in some small ways. So here are some notices.

1. I didn't know soup_session_abort. Therefore I did such wacky stuff.

2. The memory leak from env->data was fixed.

3. I removed all special stuff to handle empty messages and I removed the according FIXMEs.

4. the use of the Accept header in HttpServer? code was definitely a bug. Fixed.

5. Christopher fixed some additional bugs according libsoup 2.4 compilation and ifdef ordering.

6. After a note from Christopher I found a second use of soup_session_try_prune_connection in the http client. This was replaced with soup_session_abort and the finalize function calls now the disconnect function if a session is still present.

The actually commited stuff does not include the full patch because some tests of "make test" (devinf and manager) are failing.

03/11/08 21:05:25 changed by bellmich

  • status changed from new to assigned.

03/11/08 21:28:57 changed by bellmich

"make test" works now. Several fixes were required.

1. The finalize function of http client calls no longer the disconnect function because the disconnect function sends an event.

2. It is necessary to register the event callback before the transport layer is finally initialized because after a successful initialization it must always be possible to send/receive events. Several tests called Initialize/SetEventCallback in the wrong order.

The final SVN revision for the libsyncml patch is 409. The bug is still open because the mistake described in the second item affects the syncml plugin too!

03/11/08 21:32:48 changed by bellmich

  • status changed from assigned to closed.
  • resolution set to fixed.

I checked the syncml plugin. It is not affected by the same bug like the tests. Therefor I close this bug.

@Christopher: if you have still trouble then please re-open this bug.

@Dan: thanks for the hint towards libsoup 2.4 and the patch incl. the review of our code.