Candidate/2.8.4 #21
Merged
Candidate/2.8.4 #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add heartbeat tests
…ntation - AbstractCBLWebSocket is now thread safe - StateMachine no longer attempts to be thread safe - AbstractCBLWebSocket now handles close in CONNECTING state - AbstractCBLWebSocket state machine now correctly handles remote close request.
pasin
approved these changes
Feb 2, 2021
| socket.getHttpClient().pingIntervalMillis()); | ||
| } | ||
| finally { | ||
| repl.stop(); |
There was a problem hiding this comment.
Is it OK to not wait until the replicator's stopped before returning?
There was a problem hiding this comment.
After thought, I guessed it's OK, because the tear down will delete or close database and it will wait for the replicator to stop.
android/main/java/com/couchbase/lite/internal/CouchbaseLiteInternal.java
Show resolved
Hide resolved
borrrden
reviewed
Feb 2, 2021
common/main/cpp/native_c4base.cc
Outdated
| @@ -36,7 +36,7 @@ using namespace litecore::jni; | |||
| */ | |||
| JNIEXPORT void JNICALL Java_com_couchbase_lite_internal_core_C4Base_debug(JNIEnv *env, jclass ignore) { | |||
| c4log_enableFatalExceptionBacktrace(); | |||
| c4log_getWarnOnErrors(); | |||
| c4log_warnOnErrors(true); | |||
There was a problem hiding this comment.
I would caution that this will end up being noisy, and logging exceptions whenever they are thrown, even if they don't cause an actual issue (i.e .they are caught and handled later)
There was a problem hiding this comment.
Right. Will take it out and put it back in 3.0, protected by the "DEBUGGING" flag.
borrrden
reviewed
Feb 2, 2021
| return n; | ||
| } | ||
|
|
||
| public static long assertNotNegative(long n, @NonNull String name) { | ||
| if (n < 0) { throw new IllegalArgumentException(name + " must be negative"); } | ||
| if (n < 0) { throw new IllegalArgumentException(name + " must must not be <0"); } |
borrrden
approved these changes
Feb 2, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add heartbeat tests
CBL-1624: Fix logging bugs
Fix some bugs introduced by the state-machine based implementation of AbstractCBLWebSocket
- AbstractCBLWebSocket is now thread safe
- StateMachine no longer attempts to be thread safe
- AbstractCBLWebSocket now handles close in CONNECTING state
- AbstractCBLWebSocket state machine now correctly handles remote close request.
(Introduce a new state: CLOSE_REQUESTED and handle it properly)