Hi Tobias, and everybody,
I’ve found a little bug in the Android library, and I say little because
is not a plausible case, and also is very easy to fix, but I’m worried
if such fix could introduce worse bugs, let me explain:
When making a rpc call, the wamp connection creates a call object, and
then it fowards it to the writer and also puts it in the calls hashmap.
When the response comes, its ID is matched with the calls hashmap. So,
what’s the problem?: with an application in the emulator (and the server
in localhost), it is “very” easy to produce (by stressing it actually) a
race condition, where the call is written and the response comes so
quickly that its call ID is not yet recorded in the calls hashmap, so it
ah. ok. bad …
simply discards the result of the call and we never get a return value,
you should at least see a log message when the call result/error cannot
be matched to a call
You’re right, you get a warning in the catlog, but I meant that the application using the library doesn’t get anything to handle with, is silent from its point of view. I wonder if perhaps raising an exception instead of a simple warning would be better, but of course, that would introduce its own problems, for instance, it would be pretty easy to attack an application just by forking a fake call response and making it crash. So I think its ok by now.
or even an error, making it very hard to detect.
Code in the lines 288 and 289 of WampConnection looks like these:
If we simply invert the order of these lines:
looks fine for me. could you send a pull request on GitHub … I merge.
We solved the bug described previously. Not without a cost however, we
may then add objects to the hashmap, that can result in errors in the
WampWriter, so we would have a bit of a memory leak in these cases. We
may then use timeouts to wipe the calls that went wrong, or … simply
ignore them (since they will be wiped anyway in the next connection
reboot, since the hashmap belongs to the conection).
yes, I’d argue that is good enough. since when something goes wrong with
the WampWriter, it’s very likely that the cause is a loss of the
underlying WebSocket connection, which will result in a new connection
upon reconnect anyway …
I would like to note, that in the subscribe method there is a similar condition, where the writer could be faster than the hashmap, so you will probably loose a first incoming message. Although, is not really a problem, since publish/subscribe is supposed to be fully asynchronous, so such cases (of lost messages) should be handled by the application logic anyway.
Also, I think there is a little bug in the unsubscribe method, since it sends the unsubscribe message to the writer, but it never removes the subscription from the mSubs hashmap, It will be cleared in the next reconnection anyway, but for a small time span it will still receive messages from such subscription, and that might be a vulnerability.
I never had actual practical problems with these methods, but I’m letting you know just in case. I could fix those to if you want.
This is obviously not a big concern, but, what do you think? what should
That’s all, regards,
Thanks for tracking this down (and sorry for the hassles;).
No problem, thanks for such a great library, I’m using it in a couple of projects now, with very good results.
You received this message because you are subscribed to the Google
Groups “Autobahn” group.
To unsubscribe from this group and stop receiving emails from it, send
an email to autobahnws+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
On Saturday, March 16, 2013 5:05:11 AM UTC-4:30, Tobias Oberstein wrote:
Am 12.03.2013 21:42, schrieb Alejandro Hernandez: