Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions android/proguard.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
-keep public class io.ably.lib.transport.WebSocketTransport$Factory {*;}
-keep class io.ably.lib.types.** {*;}
-keep class io.ably.lib.objects.*LiveObjectsPlugin {*;}
-keep class io.ably.lib.objects.serialization.*Serializer {*;}
-keep class io.ably.lib.objects.ObjectsJsonSerializer {*;}

# LiveObjects implementations are resolved at runtime via Class.forName(...) +
# getDeclaredConstructor(...).newInstance(...) against hard-coded class-name strings
# (see LiveObjectsPlugin.Factory, ObjectSerializer.Holder, LiveCounter#create and
# LiveMap#create). R8 must not rename or strip these classes or their reflectively
# invoked constructors.
-keep class io.ably.lib.liveobjects.DefaultLiveObjectsPlugin { <init>(...); }
-keep class io.ably.lib.liveobjects.serialization.DefaultObjectsSerializer { <init>(...); }
-keep class io.ably.lib.liveobjects.value.livecounter.DefaultLiveCounter { <init>(...); }
-keep class io.ably.lib.liveobjects.value.livemap.DefaultLiveMap { <init>(...); }

# ObjectJsonSerializer and WireObjectDataJsonSerializer are instantiated reflectively
# by Gson via @JsonAdapter annotations (on ProtocolMessage and WireObjectData
# respectively); keep their no-arg constructors.
-keep class io.ably.lib.liveobjects.serialization.ObjectJsonSerializer { <init>(...); }
-keep class io.ably.lib.liveobjects.serialization.WireObjectDataJsonSerializer { <init>(...); }

# The Wire* object model is (de)serialized to/from JSON by Gson via field-name
# reflection (DefaultObjectsSerializer -> JsonSerialization.gson.fromJson/toJsonTree).
# R8 must not rename or strip these fields or JSON transport silently breaks. Mirrors
# the io.ably.lib.types.** rule above. (The MessagePack path uses explicit string keys
# and is unaffected.)
-keep class io.ably.lib.liveobjects.message.** { *; }

-keep class org.msgpack.core.** {*;}
-keepclasseswithmembers class io.ably.lib.rest.Auth** {*;}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/**
* Represents the value at a given key in a {@code LiveMap} object.
*
* <p>Spec: ME1
* <p>Spec: OME1
*/
public interface ObjectsMapEntry {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
*/
public abstract class LiveCounter {

private static final String IMPLEMENTATION_CLASS = "io.ably.lib.liveobjects.value.DefaultLiveCounter";
private static final String IMPLEMENTATION_CLASS = "io.ably.lib.liveobjects.value.livecounter.DefaultLiveCounter";

/**
* Extended by the LiveObjects implementation; not intended for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*/
public abstract class LiveMap {

private static final String IMPLEMENTATION_CLASS = "io.ably.lib.liveobjects.value.DefaultLiveMap";
private static final String IMPLEMENTATION_CLASS = "io.ably.lib.liveobjects.value.livemap.DefaultLiveMap";

/**
* Extended by the LiveObjects implementation; not intended for
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.ably.lib.liveobjects

import io.ably.lib.liveobjects.adapter.AblyClientAdapter
import io.ably.lib.realtime.ChannelState
import io.ably.lib.types.ProtocolMessage
import java.util.concurrent.ConcurrentHashMap

public class DefaultLiveObjectsPlugin(private val adapter: AblyClientAdapter) : LiveObjectsPlugin {

private val objects = ConcurrentHashMap<String, DefaultRealtimeObject>()

override fun getInstance(channelName: String): RealtimeObject {
return objects.getOrPut(channelName) { DefaultRealtimeObject(channelName, adapter) }
}
Comment on lines +10 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the plugin and object lifecycle code referenced in the comment.
git ls-files 'liveobjects/src/main/kotlin/io/ably/lib/liveobjects/*.kt'
echo '---'
sed -n '1,120p' liveobjects/src/main/kotlin/io/ably/lib/liveobjects/DefaultLiveObjectsPlugin.kt
echo '---'
sed -n '1,220p' liveobjects/src/main/kotlin/io/ably/lib/liveobjects/DefaultRealtimeObject.kt
echo '---'
sed -n '1,220p' liveobjects/src/main/kotlin/io/ably/lib/liveobjects/RealtimeObject.kt
echo '---'
rg -n "getOrPut|computeIfAbsent|dispose\(|incomingObjectsHandler|launch\(" liveobjects/src/main/kotlin/io/ably/lib/liveobjects

Repository: ably/ably-java

Length of output: 14405


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the remaining lifecycle methods and locate the RealtimeObject interface.
sed -n '220,420p' liveobjects/src/main/kotlin/io/ably/lib/liveobjects/DefaultRealtimeObject.kt
echo '---'
fd -a 'RealtimeObject.kt' .
echo '---'
rg -n "interface RealtimeObject|class DefaultRealtimeObject|dispose\(" liveobjects/src/main/kotlin/io/ably/lib/liveobjects

Repository: ably/ably-java

Length of output: 8199


🌐 Web query:

Kotlin stdlib ConcurrentMap getOrPut defaultValue may be invoked even if key is already in the map documentation

💡 Result:

Yes, it is explicitly documented that the defaultValue function for ConcurrentMap.getOrPut in the Kotlin standard library may be invoked even if the key is already present in the map [1][2][3]. The official Kotlin documentation states: "This method guarantees not to put the value into the map if the key is already there, but the defaultValue function may be invoked even if the key is already in the map" [2][4][3]. This behavior occurs because of the implementation of the function [1][3]: public inline fun <K, V> ConcurrentMap<K, V>.getOrPut(key: K, defaultValue: -> V): V { // Do not use computeIfAbsent on JVM8 as it would change locking behavior return this.get(key)?: defaultValue.let { default -> this.putIfAbsent(key, default)?: default } } As shown above, the function first attempts to retrieve the value using get(key) [1][3]. If the key is missing or mapped to null, it proceeds to invoke the defaultValue function before attempting to put the result into the map using putIfAbsent [1][3]. Because this check and the subsequent invocation are not performed within a single atomic operation, the defaultValue function may be called unnecessarily if another thread inserts the key into the map after the initial get(key) call but before the defaultValue function is executed [5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect build configuration for Java/Kotlin target compatibility.
git ls-files '*build.gradle*' '*settings.gradle*' 'gradle.properties' 'gradle/libs.versions.toml' 'pom.xml'
echo '---'
rg -n "sourceCompatibility|targetCompatibility|jvmTarget|kotlinOptions|toolchain|javaVersion|JavaVersion|kotlin.*jvm|org.jetbrains.kotlin.jvm|jvmToolchain" \
  -g '!*build*' -g '!**/build/**' \
  .

Repository: ably/ably-java

Length of output: 1287


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' liveobjects/build.gradle.kts
echo '---'
sed -n '1,220p' build.gradle.kts
echo '---'
sed -n '1,220p' gradle.properties

Repository: ably/ably-java

Length of output: 3505


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "JavaVersion\.VERSION_|sourceCompatibility|targetCompatibility|jvmTarget|jvmToolchain|toolchain|VERSION_1_8|VERSION_11|VERSION_17" \
  liveobjects build.gradle.kts gradle.properties gradle/libs.versions.toml java build.gradle.kts

Repository: ably/ably-java

Length of output: 302


Avoid getOrPut here; concurrent first-time calls can start and drop extra DefaultRealtimeObject instances.

ConcurrentMap.getOrPut may invoke the factory even when another thread wins the race, and DefaultRealtimeObject starts a long-lived coroutine in init. That leaves losing instances undisposed. Use computeIfAbsent so only one object is constructed per channel.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@liveobjects/src/main/kotlin/io/ably/lib/liveobjects/DefaultLiveObjectsPlugin.kt`
around lines 10 - 14, The getInstance implementation in DefaultLiveObjectsPlugin
uses ConcurrentHashMap.getOrPut, which can create extra DefaultRealtimeObject
instances during concurrent first access and leave losing instances undisposed.
Replace the getOrPut usage with ConcurrentMap.computeIfAbsent on objects so only
one DefaultRealtimeObject is constructed per channel, and keep the caching
behavior inside getInstance unchanged.


override fun handle(msg: ProtocolMessage) {
val channelName = msg.channel
objects[channelName]?.handle(msg)
}

override fun handleStateChange(channelName: String, state: ChannelState, hasObjects: Boolean) {
objects[channelName]?.handleStateChange(state, hasObjects)
}

override fun dispose(channelName: String) {
objects.remove(channelName)
?.dispose(clientError("Channel has been released using channels.release()"))
}

override fun dispose() {
objects.values.forEach {
it.dispose(clientError("AblyClient has been closed using client.close()"))
}
objects.clear()
}
}
Loading
Loading