| Age | Commit message (Collapse) | Author |
|---|
|
Add the missing FileTransfer tests to stroke. When porting the tests
I found some of them were failing and required changes to the classes
being tested to fix. Had to add a DummyNetworkEnvironment as well for
the OutgoingJingleFileTransferTest.
Test-information:
All unit tests pass.
Change-Id: Id511a556ef3a5d66e0e107f36f736db3bbb3a437
|
|
Use and ArrayList<byte[]> as the internal storage, instead of
Vector<Byte>.
Constructors and methods that supply a byte[] to add to the instance
will have that value copied, rather than just taking ownership of the
byte[]. There is an argument, on efficiency grounds, for providing
methods that take ownership of the supplied argument.
The earlier discussions about use of synchronization in the class are
revisited. This is a low-level container that should be thread-safe or
take precautions against non-thread-safe use. It is anticipated that the
vast majority of uses will be thread-safe. Use of synchronization
permits occasional deviations from this without imposing any significant
overhead for the normal case - uncontended synchronization is close to
free.
Change-Id: Ifb7b2915d5c96345c53aec8e33673d892c430431
|
|
Some discussion followed the "Fix synchronization problem in
ByteArray" patch, and that led us to believe that it would be better
to change the JavaConnection class so that it does not rely on being
able to pass ByteArrays around in a way that makes them vulnerable to
the problems that had been seen.
The JavaConnection class accepts a ByteArray in its "write()" method,
and emits a ByteArray when it has read data.
ByteArrays are not the ideal way for the JavaConnection class to
manipulate data and so this patch changes the implementation so that:
a) the "write()" method extracts the byte[] from the supplied
ByteArray and uses these objects, rather than keeping
references to the ByteArray objects (which might lead to
synchronisation issues).
b) the "doRead()" method uses a ByteArrayOutputStream to hold incoming
data, and only constructs a ByteArray out of it when it is ready to
return the data to the application.
These changes make the class more efficient, since in the case of (a),
the need to create temporary ByteArrays is removed, and in (b) the
code no longer creates ByteArrays by iterating through the network
data one byte at a time and appending it to a ByteArray.
It also means that the "synchronized" patch (which would fix the
problem) is no longer necessary, and so that code is reverted.
Test-information:
I patched the code to emulate the situation that would occur when a
buffer is only partially written, and verified that in this case it
correctly re-inserted the unwritten portion of the buffer at the
front of the pending queue.
Ran MLC to various servers, all seems to work OK.
Tested in Harrier, seems to work OK, and does not exhibit problems
that we had seen previously which led us to investigate this issue.
Change-Id: Ifcda547402430c87da45ba7d692518b5af285763
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
We noticed that in certain circumstances a stream of data being sent
to a server was being corrupted.
According to the "onDataWritten" signal, we could see that the data
which Stroke thought it was writing was valid, but by adding debug
code to the JavaConnection class, we could see that what was actually
being sent over the socket was wrong. For example, where
"onDataWritten" would report something like
some text for the server
the actual data being written to the socket (as shown by
toString() of the bytestream) would be something like:
some text fo\200\300\200\300\200\300\200\300\200\300\200\300\200\300\200\300\200\300\200\300\200\300\200\300
i.e. the length of data is correct, but the last part of the buffer is broken.
We saw this on non-TLS connections, but never on TLS connections.
The reason for this (verified after some debugging) is that the
"ByteData.getData()" method was unsynchronized. In the failing cases,
two threads are calling this method at once. The first one finds that
"dataCopy_" is null, and so new's it and starts filling it with data.
The second thread calls "getData()" before this completes, which means
it sees "dataCopy_" as non-null, and uses that value (even though the
first thread hasn't finished populating it yet).
In the failing scenario, the two threads involved were (1) thread that
was handling the "onDataWritten()" callback (which called "getData()"
to get a String that it sent to a debug stream) and (2) the
JavaConnection code (which wants to write the data to the socket).
It seems likely that the reason this doesn't happen for TLS
connections is that in that case, the JavaConnection object will be
processing a ByteArray object that has been generated via the
SSLEngine (rather than the one which "onDataWritten()" sees, and so
the chance of two threads both calling "getData()" is reduced.
(I have not followed the TLS code path thoroughly to verify this).
So this change makes any method in ByteArray that touches "dataCopy_"
be synchronized (as well as hashCode() as suggested by findbugs)
Test-information:
Having inserted some debug code, I could reproduce the "data
corruption" problem reliably.
After adding the "synchronized" directive to "getData()", I could no
longer reproduce the corruption.
Ran MLC with this patch and works with no problems
Change-Id: I02008736a2a8bd44f3702c4526fd67369a3c136a
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
|
|
This change provides implementation for the previously empty
"readFromFile" method, which is based on the code inside Swiften's
ByteArray.readByteArrayFromFile().
Note that, as for the Swiften method, no errors are reported if an
attempt to read from a file fails; it just leaves the contents of the
object unchanged.
Some javadoc was also provided, especially noting that the various
String methods are only safe so long as you're working with UTF-8
Strings.
Test-information:
Existing Stroke unit tests still pass
Signed-off-by: Nick Hudson <nick.hudson@isode.com>
|
|
|
|
Also updates build.xml so the path to the xpp library can be specified,
rather than needing the same layout as my build tree.
|
|
|
Swift