dotCMS / core

Headless/Hybrid Content Management System for Enterprises
http://dotcms.com
Other
865 stars 467 forks source link

File Handles not getting cleared #23082

Closed wezell closed 2 years ago

wezell commented 2 years ago

We have a customer that is calling the /api/vtl for content with every site hit, which is fine, except we are seeing a bunch of threads - like 50 of them - hung up in this code:

"url:GET//mysite.com/api/vtl/mycontent | lang:1 | ip:98.97.161.179 | Admin:false | start:09-30-2022 04:38:51 UTC  ref:https://mysite.com/application/themes/app/workers/outageWorker.js" #28
   java.lang.Thread.State: RUNNABLE
        at sun.nio.fs.UnixNativeDispatcher.open0(Native Method)
        at sun.nio.fs.UnixNativeDispatcher.open(UnixNativeDispatcher.java:71)
        at sun.nio.fs.UnixChannelFactory.open(UnixChannelFactory.java:267)
        at sun.nio.fs.UnixChannelFactory.newFileChannel(UnixChannelFactory.java:143)
        at sun.nio.fs.UnixChannelFactory.newFileChannel(UnixChannelFactory.java:156)
        at sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:217)
        at java.nio.file.Files.newByteChannel(Files.java:371)
        at java.nio.file.Files.newByteChannel(Files.java:422)
        at java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:420)
        at java.nio.file.Files.newInputStream(Files.java:156)
        at com.dotmarketing.portlets.fileassets.business.FileAsset.getInputStream(FileAsset.java:167)
        at com.dotcms.rest.api.v1.vtl.FileVelocityReader.getVelocity(FileVelocityReader.java:68)
        at com.dotcms.rest.api.v1.vtl.VTLResource.processRequest(VTLResource.java:537)
        at com.dotcms.rest.api.v1.vtl.VTLResource.get(VTLResource.java:104)

Looking at that code, it seems that we are never calling close on the reader that we use to read the velocity file. We should close the reader with a try/close with resources.

wezell commented 2 years ago

To QA - make sure the /api/vtl endpoint still works as expected - perhaps try hitting a get.vtl with ab

fmontes commented 2 years ago

https://github.com/dotCMS/core/pull/23083

jcastro-dotcms commented 2 years ago

I was NOT able to generate a Thread Dump with a similar trace for the test Scripting API Endpoint I created. However, these are the steps I followed in my local dotCMS instance:

  1. Create a test Content Type named Employee with the following text fields: 1.1. First Name. 1.2. Last Name. 1.3. Email. 1.4. Phone.
  2. Created around 10 contents of type Employee.
  3. Created the following folder: /application/apivtl/test/ and added a get.vtl file with this code:
    
    #set($employeeList = $dotcontent.pull("+structureName:Employee +(conhost:$!{host.identifier} conhost:SYSTEM_HOST)",0,"Employee.lastName"))

set($resultList = [])

foreach($employee in $employeeList)

#set($person = {})
$person.put("firstName", $employee.firstName)
$person.put("lastName", $employee.lastName)
$person.put("email", $employee.email)
$person.put("phone", $employee.phone)
$person.put("identifier", $employee.identifier)

$resultList.add($person);

end

$dotJSON.put("employees", $resultList)


4. Checked that the endpoint is correctly returning all 10 contentlets I created: http://localhost:8080/api/vtl/test
5. Used Apache Benchmark to try to simulate the situation found with the Thread Dump with the following command: `ab -n 10000 -c 10 http://localhost:8080/api/vtl/test `. However, I never got to see a similar trace.

It's very likely that this situation is only reproducible in live customer instances, which take a lot of traffic and execute more complex Velocity code. This would make this issue very hard, if not impossible to test in a local environment.
jcastro-dotcms commented 2 years ago

Note to QA:

I also created example Endpoints for POST, PUT, PATCH and DELETE based on my previous scenario and our official documentation: https://www.dotcms.com/docs/latest/scripting-api . The Scripting API seems to be working as expected.