Closed dalabarge closed 2 years ago
@dalabarge, could you provide a reproducible code? I am having issues reproducing your problem.
Not able to produce a repeatable trace. I believe it's caused by a Java exception because the internally tracked _id
which is passed from the JS wrapper to the Java side as the cid
has been unset. I believe this is because at some point in my code I'm calling socket.end()
or .destroy()
and it must be getting called twice through an async race condition. If the JS wrapper were to catch the second .end()
call and do some safety check on it to see that _id
is no longer set, it could either noop or throw a JS exception that my code can then catch. As it is now, it doesn't do any safety check so it goes straight to Java side and then throws an uncatchable error which crashes the app. Based on that, perhaps you can trace your code to see how _id
would get unset and verify if my assumption is right. Then if it is, you could add the safety checks so that even if my code calls .end()
too many times, I at least get a catchable error. I'm working my side to see if I can avoid the exception altogether.
@dalabarge, I think it could be caused by calling end()
on a not yet connected socket. Could this be the case?
My pseudo-code flow without all the app specific stuff is as follows. I've annotated with rationale and possible issue within my close() method. That said it appears socket.destroy()
(or end()
) are not idempotent when called.
class Component
{
constructor() {
this.socket = null
this.close.bind(this)
}
connect() {
// do a bit of cleanup to ensure clean state before creation new socket
this.close()
NetInfo.fetch('wifi')
.then((net) => {
// make sure we're using wifi only
if( net.type !== 'wifi' ) return
const socket = TcpSocket.createConnection(options), () => {
// set the active socket reference for use outside connect()
this.socket = socket
// apply a connection idleness timeout
if( Platform.OS === 'android' ) {
this.socket.setTimeout(10000, this.close)
}
})
socket.on('data', (data) => {
// ...
// inside a state manipulation I do call the
// socket.setTimeout() with a different duration
// but there's no direct calls to close() to reset socket
})
socket.on('close', () => {
// need to cleanup, especially if connection dropped without error
this.close()
// ...
})
socket.on('error', (error) => {
// an error should trigger a connection close so no need to cleanup here
// ...
})
})
// anything goes wrong cleanup socket
.catch(this.close)
}
close() {
// noop indempotent guard
if( ! _.isNil(this.socket) ) {
try {
// should be safe because socket should exist
// ideally I should be able to call indempotently
this.socket.destroy()
} catch(error) {
// noop handler because not interested in errors in cleanup
}
// Possibly this line should move above the try/catch to
// ensure a race time duplicate call doesn't also
// attempt to destroy the socket while inprogress already?
this.socket = null
}
}
}
@dalabarge, you are right, socket.destroy()
and socket.end()
are not idempotent. I must check NodeJS' net API to make sure theirs are and change ours socket.destroy()
and socket.end()
accordingly. This might fix your issue.
:tada: This issue has been resolved in version 5.6.1 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
Getting TCP Client Receives a Non-Integer
I'm getting a significant in uptick of ANRs relating to
getTcpClient(cid)
wherecid
is not an integer:Current behavior
It appears that the
cid
passed togetTcpClient()
inTcpSocketModule.java
when callingend()
is not an integer. The JS side internally manages the_id
and therefore at some point the_id
is a non-integer triggering an exception when this package passes the_id
to theend()
ordestroy()
method as acid
. Seems to be occuring since upgrading from4.x
to5.x
Expected behavior
Only way for that to happen is for the JS to lose a reference or reset the internal
_id
reference and pass it over the bridge. The bridge should protect against such errant calls as a noop to avoid the crash.Relevant information